1
0
mirror of https://github.com/phpbb/phpbb.git synced 2025-05-10 17:45:18 +02:00

[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
This commit is contained in:
Joas Schilling 2013-04-30 15:07:56 +02:00
parent 202484ebb4
commit 529e4c00fb

View File

@ -55,6 +55,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
*/ */
protected $item_basic_data = array('*'); protected $item_basic_data = array('*');
/**
* Does the class currently have a lock on the item table?
* @var bool
*/
protected $lock_acquired = false;
/** /**
* Construct * 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); 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 * @inheritdoc
*/ */
@ -191,10 +237,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
return false; return false;
} }
if (!$this->lock->acquire()) $this->acquire_lock();
{
throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE');
}
$action = ($delta > 0) ? 'move_up' : 'move_down'; $action = ($delta > 0) ? 'move_up' : 'move_down';
$delta = abs($delta); $delta = abs($delta);
@ -210,7 +253,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
if (!$item) if (!$item)
{ {
$this->lock->release(); $this->release_lock();
throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM');
} }
@ -246,7 +289,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
if (!$target) if (!$target)
{ {
$this->lock->release(); $this->release_lock();
// The item is already on top or bottom // The item is already on top or bottom
return false; return false;
} }
@ -298,7 +341,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
" . $this->get_sql_where(); " . $this->get_sql_where();
$this->db->sql_query($sql); $this->db->sql_query($sql);
$this->lock->release(); $this->release_lock();
return true; return true;
} }
@ -337,15 +380,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM');
} }
if (!$this->lock->acquire()) $this->acquire_lock();
{
throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE');
}
$item_data = $this->get_subtree_data($current_parent_id); $item_data = $this->get_subtree_data($current_parent_id);
if (!isset($item_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'); 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) if (($current_parent[$this->column_right_id] - $current_parent[$this->column_left_id]) <= 1)
{ {
$this->lock->release(); $this->release_lock();
return false; return false;
} }
if (in_array($new_parent_id, $move_items)) if (in_array($new_parent_id, $move_items))
{ {
$this->lock->release(); $this->release_lock();
throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT');
} }
@ -385,7 +425,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
if (!$new_parent) if (!$new_parent)
{ {
$this->db->sql_transaction('rollback'); $this->db->sql_transaction('rollback');
$this->lock->release(); $this->release_lock();
throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); 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_query($sql);
$this->db->sql_transaction('commit'); $this->db->sql_transaction('commit');
$this->lock->release(); $this->release_lock();
return true; return true;
} }
@ -446,15 +486,12 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM');
} }
if (!$this->lock->acquire()) $this->acquire_lock();
{
throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE');
}
$item_data = $this->get_subtree_data($item_id); $item_data = $this->get_subtree_data($item_id);
if (!isset($item_data[$item_id])) if (!isset($item_data[$item_id]))
{ {
$this->lock->release(); $this->release_lock();
throw new OutOfBoundsException($this->message_prefix . 'INVALID_ITEM'); 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)) if (in_array($new_parent_id, $move_items))
{ {
$this->lock->release(); $this->release_lock();
throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT');
} }
@ -487,7 +524,7 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
if (!$new_parent) if (!$new_parent)
{ {
$this->db->sql_transaction('rollback'); $this->db->sql_transaction('rollback');
$this->lock->release(); $this->release_lock();
throw new OutOfBoundsException($this->message_prefix . 'INVALID_PARENT'); 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_query($sql);
$this->db->sql_transaction('commit'); $this->db->sql_transaction('commit');
$this->lock->release(); $this->release_lock();
return true; 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? * @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 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 * 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 * @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()) $acquired_new_lock = $this->acquire_lock();
{
throw new RuntimeException($this->message_prefix . 'LOCK_FAILED_ACQUIRE');
}
$diff = sizeof($subset_items) * 2; $diff = sizeof($subset_items) * 2;
$sql_subset_items = $this->db->sql_in_set($this->column_item_id, $subset_items); $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')); " . ((!$set_subset_zero) ? ' WHERE ' . $sql_not_subset_items . ' ' . $this->get_sql_where('AND') : $this->get_sql_where('WHERE'));
$this->db->sql_query($sql); $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) public function regenerate_left_right_ids($new_id, $parent_id = 0, $reset_ids = false)
{ {
if ($acquired_new_lock = $this->acquire_lock())
{
$this->db->sql_transaction('begin');
}
if ($reset_ids) 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 . ' $sql = 'UPDATE ' . $this->table_name . '
SET ' . $this->db->sql_build_array('UPDATE', array( SET ' . $this->db->sql_build_array('UPDATE', array(
$this->column_left_id => 0, $this->column_left_id => 0,
@ -817,11 +849,10 @@ abstract class phpbb_tree_nestedset implements phpbb_tree_interface
} }
$this->db->sql_freeresult($result); $this->db->sql_freeresult($result);
if ($acquired_new_lock)
if ($reset_ids)
{ {
$this->db->sql_transaction('commit'); $this->db->sql_transaction('commit');
$this->lock->release(); $this->release_lock();
} }
return $new_id; return $new_id;