From b334a2ce0fe3ae196da9686028667c430eb411d1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 25 Apr 2013 17:04:37 +0200 Subject: [ticket/11495] Move classes to tree/ as they all implement a tree PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 766 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 766 insertions(+) create mode 100644 phpBB/includes/tree/nestedset.php (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php new file mode 100644 index 0000000000..2110e1a6d2 --- /dev/null +++ b/phpBB/includes/tree/nestedset.php @@ -0,0 +1,766 @@ +sql_where) ? '' : $operator . ' ' . sprintf($this->sql_where, $column_prefix); + } + + /** + * @inheritdoc + */ + public function insert(array $additional_data) + { + $item_data = $this->reset_nestedset_values($additional_data); + + $sql = 'INSERT INTO ' . $this->table_name . ' ' . $this->db->sql_build_array('INSERT', $item_data); + $this->db->sql_query($sql); + + $item_data[$this->column_item_id] = (int) $this->db->sql_nextid(); + + return array_merge($item_data, $this->add($item_data)); + } + + /** + * @inheritdoc + */ + public function add(array $item) + { + $sql = 'SELECT MAX(' . $this->column_right_id . ') AS ' . $this->column_right_id . ' + FROM ' . $this->table_name . ' + ' . $this->get_sql_where('WHERE'); + $result = $this->db->sql_query($sql); + $current_max_right_id = (int) $this->db->sql_fetchfield($this->column_right_id); + $this->db->sql_freeresult($result); + + $update_item_data = array( + $this->column_parent_id => 0, + $this->column_left_id => $current_max_right_id + 1, + $this->column_right_id => $current_max_right_id + 2, + $this->column_item_parents => '', + ); + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->db->sql_build_array('UPDATE', $update_item_data) . ' + WHERE ' . $this->column_item_id . ' = ' . (int) $item[$this->column_item_id]; + $this->db->sql_query($sql); + + return $update_item_data; + } + + /** + * @inheritdoc + */ + public function remove($item_id) + { + $items = $this->get_children_branch_data($item_id); + $item_ids = array_keys($items); + + $this->remove_subset($item_ids, $items[$item_id]); + + return $item_ids; + } + + /** + * @inheritdoc + */ + public function delete($item_id) + { + $removed_items = $this->remove($item_id); + + $sql = 'DELETE FROM ' . $this->table_name . ' + WHERE ' . $this->db->sql_in_set($this->column_item_id, $removed_items) . ' + ' . $this->get_sql_where('AND'); + $this->db->sql_query($sql); + + return $removed_items; + } + + /** + * @inheritdoc + */ + public function move($item_id, $delta) + { + if ($delta == 0) + { + return false; + } + + if (!$this->lock->acquire()) + { + throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); + } + + $action = ($delta > 0) ? 'move_up' : 'move_down'; + $delta = abs($delta); + + // Keep $this->get_sql_where() here, to ensure we are in the right tree. + $sql = 'SELECT * + FROM ' . $this->table_name . ' + WHERE ' . $this->column_item_id . ' = ' . (int) $item_id . ' + ' . $this->get_sql_where(); + $result = $this->db->sql_query_limit($sql, $delta); + $item = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if (!$item) + { + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + + /** + * Fetch all the siblings between the item's current spot + * and where we want to move it to. If there are less than $delta + * siblings between the current spot and the target then the + * item will move as far as possible + */ + $sql = "SELECT {$this->column_item_id}, {$this->column_parent_id}, {$this->column_left_id}, {$this->column_right_id}, {$this->column_item_parents} + FROM " . $this->table_name . ' + WHERE ' . $this->column_parent_id . ' = ' . (int) $item[$this->column_parent_id] . ' + ' . $this->get_sql_where() . ' + AND '; + + if ($action == 'move_up') + { + $sql .= $this->column_right_id . ' < ' . (int) $item[$this->column_right_id] . ' ORDER BY ' . $this->column_right_id . ' DESC'; + } + else + { + $sql .= $this->column_left_id . ' > ' . (int) $item[$this->column_left_id] . ' ORDER BY ' . $this->column_left_id . ' ASC'; + } + + $result = $this->db->sql_query_limit($sql, $delta); + + $target = false; + while ($row = $this->db->sql_fetchrow($result)) + { + $target = $row; + } + $this->db->sql_freeresult($result); + + if (!$target) + { + $this->lock->release(); + // The item is already on top or bottom + return false; + } + + /** + * $left_id and $right_id define the scope of the items that are affected by the move. + * $diff_up and $diff_down are the values to substract or add to each item's left_id + * and right_id in order to move them up or down. + * $move_up_left and $move_up_right define the scope of the items that are moving + * up. Other items in the scope of ($left_id, $right_id) are considered to move down. + */ + if ($action == 'move_up') + { + $left_id = (int) $target[$this->column_left_id]; + $right_id = (int) $item[$this->column_right_id]; + + $diff_up = (int) $item[$this->column_left_id] - (int) $target[$this->column_left_id]; + $diff_down = (int) $item[$this->column_right_id] + 1 - (int) $item[$this->column_left_id]; + + $move_up_left = (int) $item[$this->column_left_id]; + $move_up_right = (int) $item[$this->column_right_id]; + } + else + { + $left_id = (int) $item[$this->column_left_id]; + $right_id = (int) $target[$this->column_right_id]; + + $diff_up = (int) $item[$this->column_right_id] + 1 - (int) $item[$this->column_left_id]; + $diff_down = (int) $target[$this->column_right_id] - (int) $item[$this->column_right_id]; + + $move_up_left = (int) $item[$this->column_right_id] + 1; + $move_up_right = (int) $target[$this->column_right_id]; + } + + // Now do the dirty job + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_left_id . ' = ' . $this->column_left_id . ' + CASE + WHEN ' . $this->column_left_id . " BETWEEN {$move_up_left} AND {$move_up_right} THEN -{$diff_up} + ELSE {$diff_down} + END, + " . $this->column_right_id . ' = ' . $this->column_right_id . ' + CASE + WHEN ' . $this->column_right_id . " BETWEEN {$move_up_left} AND {$move_up_right} THEN -{$diff_up} + ELSE {$diff_down} + END, + " . $this->column_item_parents . " = '' + WHERE + " . $this->column_left_id . " BETWEEN {$left_id} AND {$right_id} + AND " . $this->column_right_id . " BETWEEN {$left_id} AND {$right_id} + " . $this->get_sql_where(); + $this->db->sql_query($sql); + + $this->lock->release(); + + return true; + } + + /** + * @inheritdoc + */ + public function move_down($item_id) + { + return $this->move($item_id, -1); + } + + /** + * @inheritdoc + */ + public function move_up($item_id) + { + return $this->move($item_id, 1); + } + + /** + * @inheritdoc + */ + public function move_children($current_parent_id, $new_parent_id) + { + $current_parent_id = (int) $current_parent_id; + $new_parent_id = (int) $new_parent_id; + + if ($current_parent_id == $new_parent_id) + { + return false; + } + + if (!$current_parent_id) + { + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + + if (!$this->lock->acquire()) + { + throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); + } + + $item_data = $this->get_children_branch_data($current_parent_id); + if (!isset($item_data[$current_parent_id])) + { + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + + $current_parent = $item_data[$current_parent_id]; + unset($item_data[$current_parent_id]); + $move_items = array_keys($item_data); + + if (($current_parent[$this->column_right_id] - $current_parent[$this->column_left_id]) <= 1) + { + $this->lock->release(); + return false; + } + + if (in_array($new_parent_id, $move_items)) + { + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); + } + + $diff = sizeof($move_items) * 2; + $sql_exclude_moved_items = $this->db->sql_in_set($this->column_item_id, $move_items, true); + + $this->db->sql_transaction('begin'); + + $this->remove_subset($move_items, $current_parent, false, true); + + if ($new_parent_id) + { + // Retrieve new-parent again, it may have been changed... + $sql = 'SELECT * + FROM ' . $this->table_name . ' + WHERE ' . $this->column_item_id . ' = ' . $new_parent_id; + $result = $this->db->sql_query($sql); + $new_parent = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if (!$new_parent) + { + $this->db->sql_transaction('rollback'); + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); + } + + $new_right_id = $this->prepare_adding_subset($move_items, $new_parent, true); + + if ($new_right_id > $current_parent[$this->column_right_id]) + { + $diff = ' + ' . ($new_right_id - $current_parent[$this->column_right_id]); + } + else + { + $diff = ' - ' . abs($new_right_id - $current_parent[$this->column_right_id]); + } + } + else + { + $sql = 'SELECT MAX(' . $this->column_right_id . ') AS ' . $this->column_right_id . ' + FROM ' . $this->table_name . ' + WHERE ' . $sql_exclude_moved_items . ' + ' . $this->get_sql_where('AND'); + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + $diff = ' + ' . ($row[$this->column_right_id] - $current_parent[$this->column_left_id]); + } + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_left_id . ' = ' . $this->column_left_id . $diff . ', + ' . $this->column_right_id . ' = ' . $this->column_right_id . $diff . ', + ' . $this->column_parent_id . ' = ' . $this->db->sql_case($this->column_parent_id . ' = ' . $current_parent_id, $new_parent_id, $this->column_parent_id) . ', + ' . $this->column_item_parents . " = '' + WHERE " . $this->db->sql_in_set($this->column_item_id, $move_items) . ' + ' . $this->get_sql_where('AND'); + $this->db->sql_query($sql); + + $this->db->sql_transaction('commit'); + $this->lock->release(); + + return true; + } + + /** + * @inheritdoc + */ + public function change_parent($item_id, $new_parent_id) + { + $item_id = (int) $item_id; + $new_parent_id = (int) $new_parent_id; + + if ($item_id == $new_parent_id) + { + return false; + } + + if (!$item_id) + { + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + + if (!$this->lock->acquire()) + { + throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); + } + + $item_data = $this->get_children_branch_data($item_id); + if (!isset($item_data[$item_id])) + { + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + + $item = $item_data[$item_id]; + $move_items = array_keys($item_data); + + if (in_array($new_parent_id, $move_items)) + { + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); + } + + $diff = sizeof($move_items) * 2; + $sql_exclude_moved_items = $this->db->sql_in_set($this->column_item_id, $move_items, true); + + $this->db->sql_transaction('begin'); + + $this->remove_subset($move_items, $item, false, true); + + if ($new_parent_id) + { + // Retrieve new-parent again, it may have been changed... + $sql = 'SELECT * + FROM ' . $this->table_name . ' + WHERE ' . $this->column_item_id . ' = ' . $new_parent_id; + $result = $this->db->sql_query($sql); + $new_parent = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + if (!$new_parent) + { + $this->db->sql_transaction('rollback'); + $this->lock->release(); + throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); + } + + $new_right_id = $this->prepare_adding_subset($move_items, $new_parent, true); + + if ($new_right_id > (int) $item[$this->column_right_id]) + { + $diff = ' + ' . ($new_right_id - (int) $item[$this->column_right_id] - 1); + } + else + { + $diff = ' - ' . abs($new_right_id - (int) $item[$this->column_right_id] - 1); + } + } + else + { + $sql = 'SELECT MAX(' . $this->column_right_id . ') AS ' . $this->column_right_id . ' + FROM ' . $this->table_name . ' + WHERE ' . $sql_exclude_moved_items . ' + ' . $this->get_sql_where('AND'); + $result = $this->db->sql_query($sql); + $row = $this->db->sql_fetchrow($result); + $this->db->sql_freeresult($result); + + $diff = ' + ' . ($row[$this->column_right_id] - (int) $item[$this->column_left_id] + 1); + } + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_left_id . ' = ' . $this->column_left_id . $diff . ', + ' . $this->column_right_id . ' = ' . $this->column_right_id . $diff . ', + ' . $this->column_parent_id . ' = ' . $this->db->sql_case($this->column_item_id . ' = ' . $item_id, $new_parent_id, $this->column_parent_id) . ', + ' . $this->column_item_parents . " = '' + WHERE " . $this->db->sql_in_set($this->column_item_id, $move_items) . ' + ' . $this->get_sql_where('AND'); + $this->db->sql_query($sql); + + $this->db->sql_transaction('commit'); + $this->lock->release(); + + return true; + } + + /** + * @inheritdoc + */ + public function get_full_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 . ' + OR 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_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 item (matching the given item id) 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.* + FROM ' . $this->table_name . ' i1 + LEFT JOIN ' . $this->table_name . " i2 + ON (($condition) " . $this->get_sql_where('AND', 'i2.') . ') + WHERE i1.' . $this->column_item_id . ' = ' . (int) $item_id . ' + ' . $this->get_sql_where('AND', 'i1.') . ' + ORDER BY i2.' . $this->column_left_id . ' ' . ($order_desc ? 'ASC' : 'DESC'); + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + if (!$include_item && $item_id == $row[$this->column_item_id]) + { + continue; + } + + $rows[(int) $row[$this->column_item_id]] = $row; + } + $this->db->sql_freeresult($result); + + return $rows; + } + + /** + * Get base information of parent items + * + * Data is cached in the item_parents column in the item table + * + * @inheritdoc + */ + public function get_parent_data(array $item) + { + $parents = array(); + if ((int) $item[$this->column_parent_id]) + { + if (!$item[$this->column_item_parents]) + { + $sql = 'SELECT ' . implode(', ', $this->item_basic_data) . ' + FROM ' . $this->table_name . ' + WHERE ' . $this->column_left_id . ' < ' . (int) $item[$this->column_left_id] . ' + AND ' . $this->column_right_id . ' > ' . (int) $item[$this->column_right_id] . ' + ' . $this->get_sql_where('AND') . ' + ORDER BY ' . $this->column_left_id . ' ASC'; + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $parents[$row[$this->column_item_id]] = $row; + } + $this->db->sql_freeresult($result); + + $item_parents = serialize($parents); + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_item_parents . " = '" . $this->db->sql_escape($item_parents) . "' + WHERE " . $this->column_parent_id . ' = ' . (int) $item[$this->column_parent_id]; + $this->db->sql_query($sql); + } + else + { + $parents = unserialize($item[$this->column_item_parents]); + } + } + + return $parents; + } + + /** + * Remove a subset from the nested set + * + * @param array $subset_items Subset of items to remove + * @param array $bounding_item Item containing the right bound of the subset + * @param bool $set_subset_zero Should the parent, left and right id of the items be set to 0, or kept unchanged? + * In case of removing an item from the tree, we should the values to 0 + * In case of moving an item, we shouldkeep the original values, in order to allow "+ diff" later + * @param bool $table_already_locked Is the table already locked, or should we acquire a new lock? + * @return null + */ + protected function remove_subset(array $subset_items, array $bounding_item, $set_subset_zero = true, $table_already_locked = false) + { + if (!$table_already_locked && !$this->lock->acquire()) + { + throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); + } + + $diff = sizeof($subset_items) * 2; + $sql_subset_items = $this->db->sql_in_set($this->column_item_id, $subset_items); + $sql_not_subset_items = $this->db->sql_in_set($this->column_item_id, $subset_items, true); + + $sql_is_parent = $this->column_left_id . ' <= ' . (int) $bounding_item[$this->column_right_id] . ' + AND ' . $this->column_right_id . ' >= ' . (int) $bounding_item[$this->column_right_id]; + + $sql_is_right = $this->column_left_id . ' > ' . (int) $bounding_item[$this->column_right_id]; + + $set_left_id = $this->db->sql_case($sql_is_right, $this->column_left_id . ' - ' . $diff, $this->column_left_id); + $set_right_id = $this->db->sql_case($sql_is_parent . ' OR ' . $sql_is_right, $this->column_right_id . ' - ' . $diff, $this->column_right_id); + + if ($set_subset_zero) + { + $set_left_id = $this->db->sql_case($sql_subset_items, 0, $set_left_id); + $set_right_id = $this->db->sql_case($sql_subset_items, 0, $set_right_id); + } + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_left_id . ' = ' . $set_left_id . ', + ' . $this->column_right_id . ' = ' . $set_right_id . ', + ' . (($set_subset_zero) ? $this->column_parent_id . ' = ' . $this->db->sql_case($sql_subset_items, 0, $this->column_parent_id) . ',' : '') . ' + ' . $this->column_item_parents . " = '' + " . ((!$set_subset_zero) ? ' WHERE ' . $sql_not_subset_items . ' ' . $this->get_sql_where('AND') : $this->get_sql_where('WHERE')); + $this->db->sql_query($sql); + + if (!$table_already_locked) + { + $this->lock->release(); + } + } + + /** + * Add a subset to the nested set + * + * @param array $subset_items Subset of items to add + * @param array $new_parent Item containing the right bound of the new parent + * @return int New right id of the parent item + */ + protected function prepare_adding_subset(array $subset_items, array $new_parent) + { + $diff = sizeof($subset_items) * 2; + $sql_not_subset_items = $this->db->sql_in_set($this->column_item_id, $subset_items, true); + + $set_left_id = $this->db->sql_case($this->column_left_id . ' > ' . (int) $new_parent[$this->column_right_id], $this->column_left_id . ' + ' . $diff, $this->column_left_id); + $set_right_id = $this->db->sql_case($this->column_right_id . ' >= ' . (int) $new_parent[$this->column_right_id], $this->column_right_id . ' + ' . $diff, $this->column_right_id); + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_left_id . ' = ' . $set_left_id . ', + ' . $this->column_right_id . ' = ' . $set_right_id . ', + ' . $this->column_item_parents . " = '' + WHERE " . $sql_not_subset_items . ' + ' . $this->get_sql_where('AND'); + $this->db->sql_query($sql); + + return $new_parent[$this->column_right_id] + $diff; + } + + /** + * Resets values required for the nested set system + * + * @param array $item Original item data + * @return array Original item data + nested set defaults + */ + protected function reset_nestedset_values(array $item) + { + $item_data = array_merge($item, array( + $this->column_parent_id => 0, + $this->column_left_id => 0, + $this->column_right_id => 0, + $this->column_item_parents => '', + )); + + unset($item_data[$this->column_item_id]); + + return $item_data; + } + + /** + * @inheritdoc + */ + public function regenerate_left_right_ids($new_id, $parent_id = 0, $reset_ids = false) + { + if ($reset_ids) + { + if (!$this->lock->acquire()) + { + throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); + } + $this->db->sql_transaction('begin'); + + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->db->sql_build_array('UPDATE', array( + $this->column_left_id => 0, + $this->column_right_id => 0, + $this->column_item_parents => '', + )) . ' + ' . $this->get_sql_where('WHERE'); + $this->db->sql_query($sql); + } + + $sql = 'SELECT * + FROM ' . $this->table_name . ' + WHERE ' . $this->column_parent_id . ' = ' . (int) $parent_id . ' + ' . $this->get_sql_where('AND') . ' + ORDER BY ' . $this->column_left_id . ', ' . $this->column_item_id . ' ASC'; + $result = $this->db->sql_query($sql); + while ($row = $this->db->sql_fetchrow($result)) + { + // First we update the left_id for this module + if ($row[$this->column_left_id] != $new_id) + { + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->db->sql_build_array('UPDATE', array( + $this->column_left_id => $new_id, + $this->column_item_parents => '', + )) . ' + WHERE ' . $this->column_item_id . ' = ' . (int) $row[$this->column_item_id]; + $this->db->sql_query($sql); + } + $new_id++; + + // Then we go through any children and update their left/right id's + $new_id = $this->regenerate_left_right_ids($new_id, $row[$this->column_item_id]); + + // Then we come back and update the right_id for this module + if ($row[$this->column_right_id] != $new_id) + { + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->db->sql_build_array('UPDATE', array($this->column_right_id => $new_id)) . ' + WHERE ' . $this->column_item_id . ' = ' . (int) $row[$this->column_item_id]; + $this->db->sql_query($sql); + } + $new_id++; + } + $this->db->sql_freeresult($result); + + + if ($reset_ids) + { + $this->db->sql_transaction('commit'); + $this->lock->release(); + } + + return $new_id; + } +} -- cgit v1.2.1 From 73d873548400f18b02167ef49195a50a11970c24 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 25 Apr 2013 17:19:52 +0200 Subject: [ticket/11495] Remove fixing function from tree interface The fixing function is implementation dependent. PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 2110e1a6d2..72e3aa4d71 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -696,7 +696,24 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * @inheritdoc + * Regenerate left/right ids from parent/child relationship + * + * This method regenerates the left/right ids for the tree based on + * the parent/child relations. This function executes three queries per + * item, so it should only be called, when the set has one of the following + * problems: + * - The set has a duplicated value inside the left/right id chain + * - The set has a missing value inside the left/right id chain + * - The set has items that do not have a left/right is set + * + * When regenerating the items, the items are sorted by parent id and their + * current left id, so the current child/parent relationships are kept + * and running the function on a working set will not change any orders. + * + * @param int $new_id First left_id to be used (should start with 1) + * @param int $parent_id parent_id of the current set (default = 0) + * @param bool $reset_ids Should we reset all left_id/right_id on the first call? + * @return int $new_id The next left_id/right_id that should be used */ public function regenerate_left_right_ids($new_id, $parent_id = 0, $reset_ids = false) { -- cgit v1.2.1 From abfb7bc51fa254edd6274e39625eb8a4edec32be Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 25 Apr 2013 17:24:18 +0200 Subject: [ticket/11495] Remove add/remove from the interface PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 72e3aa4d71..8f73b9181e 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -84,9 +84,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * @inheritdoc + * Add an existing item at the end of the tree + * + * @param array $item The item to be added + * @return bool True if the item was added */ - public function add(array $item) + protected function add(array $item) { $sql = 'SELECT MAX(' . $this->column_right_id . ') AS ' . $this->column_right_id . ' FROM ' . $this->table_name . ' @@ -111,9 +114,14 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * @inheritdoc + * Remove an item from the tree WITHOUT removing the items from the table + * + * Also removes all subitems from the tree + * + * @param int $item_id The item to be deleted + * @return array Item ids that have been removed */ - public function remove($item_id) + protected function remove($item_id) { $items = $this->get_children_branch_data($item_id); $item_ids = array_keys($items); -- cgit v1.2.1 From 0def8b7d9cb06cd2abf462f18f1404fc119861bd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 25 Apr 2013 18:09:21 +0200 Subject: [ticket/11495] Use constructor arguments over properties in implementation PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 8f73b9181e..245a8165ef 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -55,6 +55,37 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ protected $item_basic_data = array('*'); + /** + * Construct + * + * @param phpbb_db_driver $db Database connection + * @param phpbb_lock_db $lock Lock class used to lock the table when moving forums around + * @param string $table_name Table name + * @param string $message_prefix Prefix for the messages thrown by exceptions + * @param string $sql_where Additional SQL restrictions for the queries + * @param array $item_basic_data Array with basic item data that is stored in item_parents + * @param array $columns Array with column names to overwrite + */ + public function __construct(phpbb_db_driver $db, phpbb_lock_db $lock, $table_name, $message_prefix = '', $sql_where = '', $item_basic_data = array(), $columns = array()) + { + $this->db = $db; + $this->lock = $lock; + + $this->table_name = $table_name; + $this->message_prefix = $message_prefix; + $this->sql_where = $sql_where; + $this->item_basic_data = (!empty($item_basic_data)) ? $item_basic_data : array('*'); + + if (!empty($columns)) + { + foreach ($columns as $column => $name) + { + $column_name = 'column_' . $column; + $this->$column_name = $name; + } + } + } + /** * Returns additional sql where restrictions * -- cgit v1.2.1 From baff4287e5a7142b7af41e56c29b064bb56fd7fb Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 25 Apr 2013 22:39:24 +0200 Subject: [ticket/11495] Fix comments and package docs PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 245a8165ef..ffe8687e54 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -1,7 +1,7 @@ Date: Fri, 26 Apr 2013 00:04:58 +0200 Subject: [ticket/11495] Make method names for add/remove more descriptive PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index ffe8687e54..ab6a9d6bb4 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -111,7 +111,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $item_data[$this->column_item_id] = (int) $this->db->sql_nextid(); - return array_merge($item_data, $this->add($item_data)); + return array_merge($item_data, $this->add_item_to_nestedset($item_data)); } /** @@ -120,7 +120,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface * @param array $item The item to be added * @return bool True if the item was added */ - protected function add(array $item) + protected function add_item_to_nestedset(array $item) { $sql = 'SELECT MAX(' . $this->column_right_id . ') AS ' . $this->column_right_id . ' FROM ' . $this->table_name . ' @@ -152,7 +152,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface * @param int $item_id The item to be deleted * @return array Item ids that have been removed */ - protected function remove($item_id) + protected function remove_item_from_nestedset($item_id) { $items = $this->get_children_branch_data($item_id); $item_ids = array_keys($items); @@ -167,7 +167,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ public function delete($item_id) { - $removed_items = $this->remove($item_id); + $removed_items = $this->remove_item_from_nestedset($item_id); $sql = 'DELETE FROM ' . $this->table_name . ' WHERE ' . $this->db->sql_in_set($this->column_item_id, $removed_items) . ' -- cgit v1.2.1 From 2afa6730232cc2e92ae6543852d031a29c8a361f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 26 Apr 2013 08:42:44 +0200 Subject: [ticket/11495] Fix doc blocks once more PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index ab6a9d6bb4..9655a08aa5 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -115,7 +115,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * Add an existing item at the end of the tree + * Add an item which already has a database row at the end of the tree * * @param array $item The item to be added * @return bool True if the item was added @@ -145,9 +145,9 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * Remove an item from the tree WITHOUT removing the items from the table + * Remove an item from the tree without deleting it from the database * - * Also removes all subitems from the tree + * Also removes all subitems from the tree without deleting them from the database either * * @param int $item_id The item to be deleted * @return array Item ids that have been removed -- cgit v1.2.1 From 8a4260703fa76bb92f144b527b3d55289568db74 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 10:32:01 +0200 Subject: [ticket/11495] Fix some docs and replace branch with other terms PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 9655a08aa5..10ab6a86e3 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -154,7 +154,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ protected function remove_item_from_nestedset($item_id) { - $items = $this->get_children_branch_data($item_id); + $items = $this->get_subtree_data($item_id); $item_ids = array_keys($items); $this->remove_subset($item_ids, $items[$item_id]); @@ -338,7 +338,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); } - $item_data = $this->get_children_branch_data($current_parent_id); + $item_data = $this->get_subtree_data($current_parent_id); if (!isset($item_data[$current_parent_id])) { $this->lock->release(); @@ -447,7 +447,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); } - $item_data = $this->get_children_branch_data($item_id); + $item_data = $this->get_subtree_data($item_id); if (!isset($item_data[$item_id])) { $this->lock->release(); @@ -529,45 +529,45 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface /** * @inheritdoc */ - public function get_full_branch_data($item_id, $order_desc = true, $include_item = true) + public function get_path_and_subtree_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 . ' OR 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); + return $this->get_set_of_nodes_data($item_id, $condition, $order_desc, $include_item); } /** * @inheritdoc */ - public function get_parent_branch_data($item_id, $order_desc = true, $include_item = true) + public function get_path_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); + return $this->get_set_of_nodes_data($item_id, $condition, $order_desc, $include_item); } /** * @inheritdoc */ - public function get_children_branch_data($item_id, $order_desc = true, $include_item = true) + public function get_subtree_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); + return $this->get_set_of_nodes_data($item_id, $condition, $order_desc, $include_item); } /** - * Get children and parent branch of the item + * Get items that are related to the given item by the condition * - * @param int $item_id The item id to get the parents/children from + * @param int $item_id The item id to get the node set 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 item (matching the given item id) 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) + protected function get_set_of_nodes_data($item_id, $condition, $order_desc = true, $include_item = true) { $rows = array(); -- cgit v1.2.1 From 4810c61fd7b912ea2914b99f7db502b6f503068f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 10:37:59 +0200 Subject: [ticket/11495] Remove get_parent_data from interface and rename it The method is implementation specific and has no use, apart from cache, that is not covered by get_path_data(). PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 10ab6a86e3..89a91ccb16 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -595,13 +595,16 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * Get base information of parent items + * Get basic data of all parent items * + * Basic data is defined in the $item_basic_data property. * Data is cached in the item_parents column in the item table * - * @inheritdoc + * @param array $item The item to get the path from + * @return array Array of items (containing basic columns from the item table) + * ID => Item data */ - public function get_parent_data(array $item) + public function get_path_basic_data(array $item) { $parents = array(); if ((int) $item[$this->column_parent_id]) -- cgit v1.2.1 From 67f2edae170576104e619ffb38672cabf44e20fa Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 13:54:50 +0200 Subject: [ticket/11495] Use descendants and ancestors instead of parents/children PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 89a91ccb16..7710895a25 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -529,32 +529,32 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface /** * @inheritdoc */ - public function get_path_and_subtree_data($item_id, $order_desc = true, $include_item = true) + public function get_path_and_subtree_data($item_id, $order_asc = true, $include_item = true) { $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; - return $this->get_set_of_nodes_data($item_id, $condition, $order_desc, $include_item); + return $this->get_set_of_nodes_data($item_id, $condition, $order_asc, $include_item); } /** * @inheritdoc */ - public function get_path_data($item_id, $order_desc = true, $include_item = true) + public function get_path_data($item_id, $order_asc = 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_set_of_nodes_data($item_id, $condition, $order_desc, $include_item); + return $this->get_set_of_nodes_data($item_id, $condition, $order_asc, $include_item); } /** * @inheritdoc */ - public function get_subtree_data($item_id, $order_desc = true, $include_item = true) + public function get_subtree_data($item_id, $order_asc = 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_set_of_nodes_data($item_id, $condition, $order_desc, $include_item); + return $this->get_set_of_nodes_data($item_id, $condition, $order_asc, $include_item); } /** @@ -562,12 +562,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface * * @param int $item_id The item id to get the node set from * @param string $condition Query string restricting the item list - * @param bool $order_desc Order the items descending (most outer parent first) + * @param bool $order_asc Order the items ascending by their left_id * @param bool $include_item Should the item (matching the given item id) be included in the list aswell * @return array Array of items (containing all columns from the item table) * ID => Item data */ - protected function get_set_of_nodes_data($item_id, $condition, $order_desc = true, $include_item = true) + protected function get_set_of_nodes_data($item_id, $condition, $order_asc = true, $include_item = true) { $rows = array(); @@ -577,7 +577,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface ON (($condition) " . $this->get_sql_where('AND', 'i2.') . ') WHERE i1.' . $this->column_item_id . ' = ' . (int) $item_id . ' ' . $this->get_sql_where('AND', 'i1.') . ' - ORDER BY i2.' . $this->column_left_id . ' ' . ($order_desc ? 'ASC' : 'DESC'); + ORDER BY i2.' . $this->column_left_id . ' ' . ($order_asc ? 'ASC' : 'DESC'); $result = $this->db->sql_query($sql); while ($row = $this->db->sql_fetchrow($result)) -- cgit v1.2.1 From 863d0c7687cc926dfda0ddd20b34e7942748fb2e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 14:36:26 +0200 Subject: [ticket/11495] Fix some more comments and the package tag PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 7710895a25..ae9805aa45 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -1,7 +1,7 @@ Date: Tue, 30 Apr 2013 14:45:22 +0200 Subject: [ticket/11495] Fix docs of add_item_to_nestedset() and take id as argument PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index ae9805aa45..934eb933e0 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -111,16 +111,17 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $item_data[$this->column_item_id] = (int) $this->db->sql_nextid(); - return array_merge($item_data, $this->add_item_to_nestedset($item_data)); + return array_merge($item_data, $this->add_item_to_nestedset($item_data[$this->column_item_id])); } /** * Add an item which already has a database row at the end of the tree * - * @param array $item The item to be added - * @return bool True if the item was added + * @param int $item_id The item to be added + * @return array Array with updated data, if the item was added successfully + * Empty array otherwise */ - protected function add_item_to_nestedset(array $item) + protected function add_item_to_nestedset($item_id) { $sql = 'SELECT MAX(' . $this->column_right_id . ') AS ' . $this->column_right_id . ' FROM ' . $this->table_name . ' @@ -138,10 +139,13 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $sql = 'UPDATE ' . $this->table_name . ' SET ' . $this->db->sql_build_array('UPDATE', $update_item_data) . ' - WHERE ' . $this->column_item_id . ' = ' . (int) $item[$this->column_item_id]; + WHERE ' . $this->column_item_id . ' = ' . (int) $item_id . ' + AND ' . $this->column_parent_id . ' = 0 + AND ' . $this->column_left_id . ' = 0 + AND ' . $this->column_right_id . ' = 0'; $this->db->sql_query($sql); - return $update_item_data; + return ($this->db->sql_affectedrows() == 1) ? $update_item_data : array(); } /** -- cgit v1.2.1 From 529e4c00fbb78ba513afbbacfb3d5465efbd82ef Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 15:07:56 +0200 Subject: [ticket/11495] Move lock code into two methods to allow easier handling This also allows to simply remove the lock handling by overwriting the two methods acquire_lock() and release_lock(). PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 111 ++++++++++++++++++++++++-------------- 1 file changed, 71 insertions(+), 40 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 934eb933e0..85e04cd1cf 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -55,6 +55,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ protected $item_basic_data = array('*'); + /** + * Does the class currently have a lock on the item table? + * @var bool + */ + protected $lock_acquired = false; + /** * Construct * @@ -99,6 +105,46 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface return (!$this->sql_where) ? '' : $operator . ' ' . sprintf($this->sql_where, $column_prefix); } + /** + * Acquires a lock on the item table + * + * @return bool True if the lock was acquired, false if it has been acquired previously + * + * @throws RuntimeException If the lock could not be acquired + */ + protected function acquire_lock() + { + if ($this->lock_acquired) + { + return false; + } + + if (!$this->lock->acquire()) + { + throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); + } + + $this->lock_acquired = true; + return true; + } + + /** + * Releases the lock on the item table + * + * @return bool False, if there was no lock to release, true otherwise + */ + protected function release_lock() + { + if (!$this->lock_acquired) + { + return false; + } + + $this->lock->release(); + $this->lock_acquired = false; + return true; + } + /** * @inheritdoc */ @@ -191,10 +237,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface return false; } - if (!$this->lock->acquire()) - { - throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); - } + $this->acquire_lock(); $action = ($delta > 0) ? 'move_up' : 'move_down'; $delta = abs($delta); @@ -210,7 +253,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$item) { - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } @@ -246,7 +289,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$target) { - $this->lock->release(); + $this->release_lock(); // The item is already on top or bottom return false; } @@ -298,7 +341,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface " . $this->get_sql_where(); $this->db->sql_query($sql); - $this->lock->release(); + $this->release_lock(); return true; } @@ -337,15 +380,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } - if (!$this->lock->acquire()) - { - throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); - } + $this->acquire_lock(); $item_data = $this->get_subtree_data($current_parent_id); if (!isset($item_data[$current_parent_id])) { - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } @@ -355,13 +395,13 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (($current_parent[$this->column_right_id] - $current_parent[$this->column_left_id]) <= 1) { - $this->lock->release(); + $this->release_lock(); return false; } if (in_array($new_parent_id, $move_items)) { - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -385,7 +425,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$new_parent) { $this->db->sql_transaction('rollback'); - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -423,7 +463,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $this->db->sql_query($sql); $this->db->sql_transaction('commit'); - $this->lock->release(); + $this->release_lock(); return true; } @@ -446,15 +486,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } - if (!$this->lock->acquire()) - { - throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); - } + $this->acquire_lock(); $item_data = $this->get_subtree_data($item_id); if (!isset($item_data[$item_id])) { - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } @@ -463,7 +500,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (in_array($new_parent_id, $move_items)) { - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -487,7 +524,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$new_parent) { $this->db->sql_transaction('rollback'); - $this->lock->release(); + $this->release_lock(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -525,7 +562,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $this->db->sql_query($sql); $this->db->sql_transaction('commit'); - $this->lock->release(); + $this->release_lock(); return true; } @@ -653,15 +690,11 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface * @param bool $set_subset_zero Should the parent, left and right id of the items be set to 0, or kept unchanged? * In case of removing an item from the tree, we should the values to 0 * In case of moving an item, we shouldkeep the original values, in order to allow "+ diff" later - * @param bool $table_already_locked Is the table already locked, or should we acquire a new lock? * @return null */ - protected function remove_subset(array $subset_items, array $bounding_item, $set_subset_zero = true, $table_already_locked = false) + protected function remove_subset(array $subset_items, array $bounding_item, $set_subset_zero = true) { - if (!$table_already_locked && !$this->lock->acquire()) - { - throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); - } + $acquired_new_lock = $this->acquire_lock(); $diff = sizeof($subset_items) * 2; $sql_subset_items = $this->db->sql_in_set($this->column_item_id, $subset_items); @@ -689,9 +722,9 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface " . ((!$set_subset_zero) ? ' WHERE ' . $sql_not_subset_items . ' ' . $this->get_sql_where('AND') : $this->get_sql_where('WHERE')); $this->db->sql_query($sql); - if (!$table_already_locked) + if ($acquired_new_lock) { - $this->lock->release(); + $this->release_lock(); } } @@ -763,14 +796,13 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ public function regenerate_left_right_ids($new_id, $parent_id = 0, $reset_ids = false) { - if ($reset_ids) + if ($acquired_new_lock = $this->acquire_lock()) { - if (!$this->lock->acquire()) - { - throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); - } $this->db->sql_transaction('begin'); + } + if ($reset_ids) + { $sql = 'UPDATE ' . $this->table_name . ' SET ' . $this->db->sql_build_array('UPDATE', array( $this->column_left_id => 0, @@ -817,11 +849,10 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } $this->db->sql_freeresult($result); - - if ($reset_ids) + if ($acquired_new_lock) { $this->db->sql_transaction('commit'); - $this->lock->release(); + $this->release_lock(); } return $new_id; -- cgit v1.2.1 From 055ee41065fb0b7c08c66daff196eb2d3460b0cc Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 15:16:41 +0200 Subject: [ticket/11495] Remove useless cast PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 85e04cd1cf..6819f9791b 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -648,7 +648,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface public function get_path_basic_data(array $item) { $parents = array(); - if ((int) $item[$this->column_parent_id]) + if ($item[$this->column_parent_id]) { if (!$item[$this->column_item_parents]) { -- cgit v1.2.1 From 78b0d3e723ab57c2e32b95a66159861fe461d77f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 15:50:14 +0200 Subject: [ticket/11495] Use $lock->owns_lock() instead of own property PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 6819f9791b..26152e6c10 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -55,12 +55,6 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ protected $item_basic_data = array('*'); - /** - * Does the class currently have a lock on the item table? - * @var bool - */ - protected $lock_acquired = false; - /** * Construct * @@ -114,7 +108,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ protected function acquire_lock() { - if ($this->lock_acquired) + if ($this->lock->owns_lock()) { return false; } @@ -124,25 +118,17 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE'); } - $this->lock_acquired = true; return true; } /** * Releases the lock on the item table * - * @return bool False, if there was no lock to release, true otherwise + * @return null */ protected function release_lock() { - if (!$this->lock_acquired) - { - return false; - } - $this->lock->release(); - $this->lock_acquired = false; - return true; } /** -- cgit v1.2.1 From 6055a3cc7ed76cfe458ea524bfad66f475f35ce8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 16:19:25 +0200 Subject: [ticket/11495] Remove useless release_lock() method PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 26152e6c10..f92606a4c4 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -121,16 +121,6 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface return true; } - /** - * Releases the lock on the item table - * - * @return null - */ - protected function release_lock() - { - $this->lock->release(); - } - /** * @inheritdoc */ @@ -239,7 +229,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$item) { - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } @@ -275,7 +265,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$target) { - $this->release_lock(); + $this->lock->release(); // The item is already on top or bottom return false; } @@ -327,7 +317,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface " . $this->get_sql_where(); $this->db->sql_query($sql); - $this->release_lock(); + $this->lock->release(); return true; } @@ -371,7 +361,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $item_data = $this->get_subtree_data($current_parent_id); if (!isset($item_data[$current_parent_id])) { - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } @@ -381,13 +371,13 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (($current_parent[$this->column_right_id] - $current_parent[$this->column_left_id]) <= 1) { - $this->release_lock(); + $this->lock->release(); return false; } if (in_array($new_parent_id, $move_items)) { - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -411,7 +401,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$new_parent) { $this->db->sql_transaction('rollback'); - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -449,7 +439,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $this->db->sql_query($sql); $this->db->sql_transaction('commit'); - $this->release_lock(); + $this->lock->release(); return true; } @@ -477,7 +467,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $item_data = $this->get_subtree_data($item_id); if (!isset($item_data[$item_id])) { - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); } @@ -486,7 +476,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (in_array($new_parent_id, $move_items)) { - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -510,7 +500,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if (!$new_parent) { $this->db->sql_transaction('rollback'); - $this->release_lock(); + $this->lock->release(); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); } @@ -548,7 +538,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $this->db->sql_query($sql); $this->db->sql_transaction('commit'); - $this->release_lock(); + $this->lock->release(); return true; } @@ -710,7 +700,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if ($acquired_new_lock) { - $this->release_lock(); + $this->lock->release(); } } @@ -838,7 +828,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if ($acquired_new_lock) { $this->db->sql_transaction('commit'); - $this->release_lock(); + $this->lock->release(); } return $new_id; -- cgit v1.2.1 From f3f7be4cd1a0495cf44787c713480d5aceb1bae1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 17:11:55 +0200 Subject: [ticket/11495] Fix @return doc of get_sql_where() PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index f92606a4c4..5a12afdfe9 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -92,7 +92,8 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface * @param string $operator SQL operator that needs to be prepended to sql_where, * if it is not empty. * @param string $column_prefix Prefix that needs to be prepended to column names - * @return bool True if the item was deleted + * @return string Returns additional where statements to narrow down the tree, + * prefixed with operator and prepended column_prefix to column names */ public function get_sql_where($operator = 'AND', $column_prefix = '') { -- cgit v1.2.1 From 5c4d69581afb9fd9b00d6b7b13cec257a97e875c Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 18:06:29 +0200 Subject: [ticket/11495] Do not reset item_parent if not required PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 5a12afdfe9..1f414164df 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -310,8 +310,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface " . $this->column_right_id . ' = ' . $this->column_right_id . ' + CASE WHEN ' . $this->column_right_id . " BETWEEN {$move_up_left} AND {$move_up_right} THEN -{$diff_up} ELSE {$diff_down} - END, - " . $this->column_item_parents . " = '' + END WHERE " . $this->column_left_id . " BETWEEN {$left_id} AND {$right_id} AND " . $this->column_right_id . " BETWEEN {$left_id} AND {$right_id} @@ -692,11 +691,10 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } $sql = 'UPDATE ' . $this->table_name . ' - SET ' . $this->column_left_id . ' = ' . $set_left_id . ', - ' . $this->column_right_id . ' = ' . $set_right_id . ', - ' . (($set_subset_zero) ? $this->column_parent_id . ' = ' . $this->db->sql_case($sql_subset_items, 0, $this->column_parent_id) . ',' : '') . ' - ' . $this->column_item_parents . " = '' - " . ((!$set_subset_zero) ? ' WHERE ' . $sql_not_subset_items . ' ' . $this->get_sql_where('AND') : $this->get_sql_where('WHERE')); + SET ' . (($set_subset_zero) ? $this->column_parent_id . ' = ' . $this->db->sql_case($sql_subset_items, 0, $this->column_parent_id) . ',' : '') . ' + ' . $this->column_left_id . ' = ' . $set_left_id . ', + ' . $this->column_right_id . ' = ' . $set_right_id . ' + ' . ((!$set_subset_zero) ? ' WHERE ' . $sql_not_subset_items . ' ' . $this->get_sql_where('AND') : $this->get_sql_where('WHERE')); $this->db->sql_query($sql); if ($acquired_new_lock) @@ -706,7 +704,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface } /** - * Add a subset to the nested set + * Prepare adding a subset to the nested set * * @param array $subset_items Subset of items to add * @param array $new_parent Item containing the right bound of the new parent @@ -722,9 +720,8 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface $sql = 'UPDATE ' . $this->table_name . ' SET ' . $this->column_left_id . ' = ' . $set_left_id . ', - ' . $this->column_right_id . ' = ' . $set_right_id . ', - ' . $this->column_item_parents . " = '' - WHERE " . $sql_not_subset_items . ' + ' . $this->column_right_id . ' = ' . $set_right_id . ' + WHERE ' . $sql_not_subset_items . ' ' . $this->get_sql_where('AND'); $this->db->sql_query($sql); @@ -776,6 +773,14 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if ($acquired_new_lock = $this->acquire_lock()) { $this->db->sql_transaction('begin'); + + if (!$reset_ids) + { + $sql = 'UPDATE ' . $this->table_name . ' + SET ' . $this->column_item_parents . " = '' + " . $this->get_sql_where('WHERE'); + $this->db->sql_query($sql); + } } if ($reset_ids) @@ -802,10 +807,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface if ($row[$this->column_left_id] != $new_id) { $sql = 'UPDATE ' . $this->table_name . ' - SET ' . $this->db->sql_build_array('UPDATE', array( - $this->column_left_id => $new_id, - $this->column_item_parents => '', - )) . ' + SET ' . $this->db->sql_build_array('UPDATE', array($this->column_left_id => $new_id)) . ' WHERE ' . $this->column_item_id . ' = ' . (int) $row[$this->column_item_id]; $this->db->sql_query($sql); } -- cgit v1.2.1 From 98e6207c35910bf9761433e62ca5fe5af3459873 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 18:13:32 +0200 Subject: [ticket/11495] Fix "as well" typo and remove brackets PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 1f414164df..2a639d8907 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -580,7 +580,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface * @param int $item_id The item id to get the node set from * @param string $condition Query string restricting the item list * @param bool $order_asc Order the items ascending by their left_id - * @param bool $include_item Should the item (matching the given item id) be included in the list aswell + * @param bool $include_item Should the item matching the given item id be included in the list as well * @return array Array of items (containing all columns from the item table) * ID => Item data */ -- cgit v1.2.1 From 6a7378ecbdeea1ad356cbde8a085aa7510c61788 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 18:39:22 +0200 Subject: [ticket/11495] Some more doc changes PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 2a639d8907..2f17ebab02 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -577,7 +577,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface /** * Get items that are related to the given item by the condition * - * @param int $item_id The item id to get the node set from + * @param int $item_id Id of the item to retrieve the node set from * @param string $condition Query string restricting the item list * @param bool $order_asc Order the items ascending by their left_id * @param bool $include_item Should the item matching the given item id be included in the list as well -- cgit v1.2.1 From d7787682df0474b7bb78434339f8edde3727c580 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Apr 2013 22:19:35 +0200 Subject: [ticket/11495] Throw exception when item that should be deleted does not exist PHPBB3-11495 --- phpBB/includes/tree/nestedset.php | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'phpBB/includes/tree/nestedset.php') diff --git a/phpBB/includes/tree/nestedset.php b/phpBB/includes/tree/nestedset.php index 2f17ebab02..4d851a87a8 100644 --- a/phpBB/includes/tree/nestedset.php +++ b/phpBB/includes/tree/nestedset.php @@ -181,9 +181,20 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface */ protected function remove_item_from_nestedset($item_id) { + $item_id = (int) $item_id; + if (!$item_id) + { + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + $items = $this->get_subtree_data($item_id); $item_ids = array_keys($items); + if (empty($items) || !isset($items[$item_id])) + { + throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); + } + $this->remove_subset($item_ids, $items[$item_id]); return $item_ids; -- cgit v1.2.1