[ticket/11495] Split get_branch_data into multiple methods

PHPBB3-11495
author: Joas Schilling <nickvergessen@gmx.de> 2013-04-25 14:05:41 +0200
committer: Joas Schilling <nickvergessen@gmx.de> 2013-04-25 14:05:41 +0200
commit: fe97915fc91e4fbfb211e3eb6038dd1b482553dd (patch)
tree: 8300aa8a54861a310e026ad8ca905e8ee70e1033
parent: ab7054445fbf397ae5bf4c13eb44a2019d71cc2d (diff)
download: forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar
forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar.gz
forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar.bz2
forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar.xz
forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.zip
3 files changed, 128 insertions, 61 deletions
diff --git a/phpBB/includes/nestedset/base.php b/phpBB/includes/nestedset/base.php
index 3817a6d217..fe6318304d 100644
--- a/phpBB/includes/nestedset/base.php
+++ b/phpBB/includes/nestedset/base.php
@@ -115,7 +115,7 @@ abstract class phpbb_nestedset_base implements phpbb_nestedset_interface
 	*/
 	public function remove($item_id)
 	{
-		$items = $this->get_branch_data($item_id, 'children');
+		$items = $this->get_children_branch_data($item_id);
 		$item_ids = array_keys($items);
 
 		$this->remove_subset($item_ids, $items[$item_id]);
@@ -299,7 +299,7 @@ abstract class phpbb_nestedset_base implements phpbb_nestedset_interface
 			throw new phpbb_nestedset_exception($this->message_prefix . 'LOCK_FAILED_ACQUIRE');
 		}
 
-		$item_data = $this->get_branch_data($current_parent_id, 'children');
+		$item_data = $this->get_children_branch_data($current_parent_id);
 		if (!isset($item_data[$current_parent_id]))
 		{
 			$this->lock->release();
@@ -408,7 +408,7 @@ abstract class phpbb_nestedset_base implements phpbb_nestedset_interface
 			throw new phpbb_nestedset_exception($this->message_prefix . 'LOCK_FAILED_ACQUIRE');
 		}
 
-		$item_data = $this->get_branch_data($item_id, 'children');
+		$item_data = $this->get_children_branch_data($item_id);
 		if (!isset($item_data[$item_id]))
 		{
 			$this->lock->release();
@@ -490,24 +490,46 @@ abstract class phpbb_nestedset_base implements phpbb_nestedset_interface
 	/**
 	* @inheritdoc
 	*/
-	public function get_branch_data($item_id, $type = 'all', $order_desc = true, $include_item = true)
+	public function get_full_branch_data($item_id, $order_desc = true, $include_item = true)
 	{
-		switch ($type)
-		{
-			case 'parents':
-				$condition = 'i1.' . $this->column_left_id . ' BETWEEN i2.' . $this->column_left_id . ' AND i2.' . $this->column_right_id . '';
-			break;
+		$condition = 'i2.' . $this->column_left_id . ' BETWEEN i1.' . $this->column_left_id . ' AND i1.' . $this->column_right_id . '
+			OR i1.' . $this->column_left_id . ' BETWEEN i2.' . $this->column_left_id . ' AND i2.' . $this->column_right_id;
 
-			case 'children':
-				$condition = 'i2.' . $this->column_left_id . ' BETWEEN i1.' . $this->column_left_id . ' AND i1.' . $this->column_right_id . '';
-			break;
+		return $this->get_branch_data($item_id, $condition, $order_desc, $include_item);
+	}
 
-			default:
-				$condition = 'i2.' . $this->column_left_id . ' BETWEEN i1.' . $this->column_left_id . ' AND i1.' . $this->column_right_id . '
-					OR i1.' . $this->column_left_id . ' BETWEEN i2.' . $this->column_left_id . ' AND i2.' . $this->column_right_id;
-			break;
-		}
+	/**
+	* @inheritdoc
+	*/
+	public function get_parent_branch_data($item_id, $order_desc = true, $include_item = true)
+	{
+		$condition = 'i1.' . $this->column_left_id . ' BETWEEN i2.' . $this->column_left_id . ' AND i2.' . $this->column_right_id . '';
+
+		return $this->get_branch_data($item_id, $condition, $order_desc, $include_item);
+	}
+
+	/**
+	* @inheritdoc
+	*/
+	public function get_children_branch_data($item_id, $order_desc = true, $include_item = true)
+	{
+		$condition = 'i2.' . $this->column_left_id . ' BETWEEN i1.' . $this->column_left_id . ' AND i1.' . $this->column_right_id . '';
+
+		return $this->get_branch_data($item_id, $condition, $order_desc, $include_item);
+	}
 
+	/**
+	* Get children and parent branch of the item
+	*
+	* @param int		$item_id		The item id to get the parents/children from
+	* @param string		$condition		Query string restricting the item list
+	* @param bool		$order_desc		Order the items descending (most outer parent first)
+	* @param bool		$include_item	Should the given item be included in the list aswell
+	* @return array			Array of items (containing all columns from the item table)
+	*							ID => Item data
+	*/
+	protected function get_branch_data($item_id, $condition, $order_desc = true, $include_item = true)
+	{
 		$rows = array();
 
 		$sql = 'SELECT i2.*
diff --git a/phpBB/includes/nestedset/interface.php b/phpBB/includes/nestedset/interface.php
index a2dbc55c7d..ee78016425 100644
--- a/phpBB/includes/nestedset/interface.php
+++ b/phpBB/includes/nestedset/interface.php
@@ -106,18 +106,37 @@ interface phpbb_nestedset_interface
 	public function change_parent($item, $new_parent_id);
 
 	/**
-	* Get branch of the item
+	* Get children and parent branch of the item
 	*
-	* This method can return all parents, children or both of the given item
+	* @param int		$item_id		The item id to get the parents/children from
+	* @param bool		$order_desc		Order the items descending (most outer parent first)
+	* @param bool		$include_item	Should the given item be included in the list aswell
+	* @return array			Array of items (containing all columns from the item table)
+	*							ID => Item data
+	*/
+	public function get_full_branch_data($item_id, $order_desc, $include_item);
+
+	/**
+	* Get parent branch of the item
 	*
 	* @param int		$item_id		The item id to get the parents from
-	* @param string		$type			One of all|parent|children
 	* @param bool		$order_desc		Order the items descending (most outer parent first)
 	* @param bool		$include_item	Should the given item be included in the list aswell
 	* @return array			Array of items (containing all columns from the item table)
 	*							ID => Item data
 	*/
-	public function get_branch_data($item_id, $type, $order_desc, $include_item);
+	public function get_parent_branch_data($item_id, $order_desc, $include_item);
+
+	/**
+	* Get children branch of the item
+	*
+	* @param int		$item_id		The item id to get the children from
+	* @param bool		$order_desc		Order the items descending (most outer parent first)
+	* @param bool		$include_item	Should the given item be included in the list aswell
+	* @return array			Array of items (containing all columns from the item table)
+	*							ID => Item data
+	*/
+	public function get_children_branch_data($item_id, $order_desc, $include_item);
 
 	/**
 	* Get base information of parent items
diff --git a/tests/nestedset/set_forum_get_data_test.php b/tests/nestedset/set_forum_get_data_test.php
index 2c8889d1a5..a0e255b9b8 100644
--- a/tests/nestedset/set_forum_get_data_test.php
+++ b/tests/nestedset/set_forum_get_data_test.php
@@ -11,62 +11,88 @@ require_once dirname(__FILE__) . '/set_forum_base.php';
 
 class phpbb_tests_nestedset_set_forum_get_data_test extends phpbb_tests_nestedset_set_forum_base
 {
-	public function get_branch_data_data()
+	public function get_full_branch_data_data()
 	{
 		return array(
-			array(1, 'all', true, true, array(1, 2, 3)),
-			array(1, 'all', true, false, array(2, 3)),
-			array(1, 'all', false, true, array(3, 2, 1)),
-			array(1, 'all', false, false, array(3, 2)),
+			array(1, true, true, array(1, 2, 3)),
+			array(1, true, false, array(2, 3)),
+			array(1, false, true, array(3, 2, 1)),
+			array(1, false, false, array(3, 2)),
 
-			array(1, 'parents', true, true, array(1)),
-			array(1, 'parents', true, false, array()),
-			array(1, 'parents', false, true, array(1)),
-			array(1, 'parents', false, false, array()),
+			array(2, true, true, array(1, 2)),
+			array(2, true, false, array(1)),
+			array(2, false, true, array(2, 1)),
+			array(2, false, false, array(1)),
 
-			array(1, 'children', true, true, array(1, 2, 3)),
-			array(1, 'children', true, false, array(2, 3)),
-			array(1, 'children', false, true, array(3, 2, 1)),
-			array(1, 'children', false, false, array(3, 2)),
+			array(5, true, true, array(4, 5, 6)),
+			array(5, true, false, array(4, 6)),
+			array(5, false, true, array(6, 5, 4)),
+			array(5, false, false, array(6, 4)),
+		);
+	}
+
+	/**
+	* @dataProvider get_full_branch_data_data
+	*/
+	public function test_get_full_branch_data($forum_id, $order_desc, $include_item, $expected)
+	{
+		$this->assertEquals($expected, array_keys($this->set->get_full_branch_data($forum_id, $order_desc, $include_item)));
+	}
+
+	public function get_parent_branch_data_data()
+	{
+		return array(
+			array(1, true, true, array(1)),
+			array(1, true, false, array()),
+			array(1, false, true, array(1)),
+			array(1, false, false, array()),
 
-			array(2, 'all', true, true, array(1, 2)),
-			array(2, 'all', true, false, array(1)),
-			array(2, 'all', false, true, array(2, 1)),
-			array(2, 'all', false, false, array(1)),
+			array(2, true, true, array(1, 2)),
+			array(2, true, false, array(1)),
+			array(2, false, true, array(2, 1)),
+			array(2, false, false, array(1)),
 
-			array(2, 'parents', true, true, array(1, 2)),
-			array(2, 'parents', true, false, array(1)),
-			array(2, 'parents', false, true, array(2, 1)),
-			array(2, 'parents', false, false, array(1)),
+			array(5, true, true, array(4, 5)),
+			array(5, true, false, array(4)),
+			array(5, false, true, array(5, 4)),
+			array(5, false, false, array(4)),
+		);
+	}
 
-			array(2, 'children', true, true, array(2)),
-			array(2, 'children', true, false, array()),
-			array(2, 'children', false, true, array(2)),
-			array(2, 'children', false, false, array()),
+	/**
+	* @dataProvider get_parent_branch_data_data
+	*/
+	public function test_get_parent_branch_data($forum_id, $order_desc, $include_item, $expected)
+	{
+		$this->assertEquals($expected, array_keys($this->set->get_parent_branch_data($forum_id, $order_desc, $include_item)));
+	}
 
-			array(5, 'all', true, true, array(4, 5, 6)),
-			array(5, 'all', true, false, array(4, 6)),
-			array(5, 'all', false, true, array(6, 5, 4)),
-			array(5, 'all', false, false, array(6, 4)),
+	public function get_children_branch_data_data()
+	{
+		return array(
+			array(1, true, true, array(1, 2, 3)),
+			array(1, true, false, array(2, 3)),
+			array(1, false, true, array(3, 2, 1)),
+			array(1, false, false, array(3, 2)),
 
-			array(5, 'parents', true, true, array(4, 5)),
-			array(5, 'parents', true, false, array(4)),
-			array(5, 'parents', false, true, array(5, 4)),
-			array(5, 'parents', false, false, array(4)),
+			array(2, true, true, array(2)),
+			array(2, true, false, array()),
+			array(2, false, true, array(2)),
+			array(2, false, false, array()),
 
-			array(5, 'children', true, true, array(5, 6)),
-			array(5, 'children', true, false, array(6)),
-			array(5, 'children', false, true, array(6, 5)),
-			array(5, 'children', false, false, array(6)),
+			array(5, true, true, array(5, 6)),
+			array(5, true, false, array(6)),
+			array(5, false, true, array(6, 5)),
+			array(5, false, false, array(6)),
 		);
 	}
 
 	/**
-	* @dataProvider get_branch_data_data
+	* @dataProvider get_children_branch_data_data
 	*/
-	public function test_get_branch_data($forum_id, $type, $order_desc, $include_item, $expected)
+	public function test_get_children_branch_data($forum_id, $order_desc, $include_item, $expected)
 	{
-		$this->assertEquals($expected, array_keys($this->set->get_branch_data($forum_id, $type, $order_desc, $include_item)));
+		$this->assertEquals($expected, array_keys($this->set->get_children_branch_data($forum_id, $order_desc, $include_item)));
 	}
 
 	public function get_parent_data_data()
author	Joas Schilling <nickvergessen@gmx.de>	2013-04-25 14:05:41 +0200
committer	Joas Schilling <nickvergessen@gmx.de>	2013-04-25 14:05:41 +0200
commit	fe97915fc91e4fbfb211e3eb6038dd1b482553dd (patch)
tree	8300aa8a54861a310e026ad8ca905e8ee70e1033
parent	ab7054445fbf397ae5bf4c13eb44a2019d71cc2d (diff)
download	forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar.gz forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar.bz2 forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.tar.xz forums-fe97915fc91e4fbfb211e3eb6038dd1b482553dd.zip