From fd6ee50e06cb48c9e3a476bf23285875484ff5f7 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Fri, 2 Nov 2012 16:05:53 -0400 Subject: [PATCH 01/22] [ticket/11162] Extract existing behavior into a function and add a test. PHPBB3-11162 --- phpBB/includes/functions.php | 24 ++++++- tests/functions/fixtures/duplicates.xml | 56 +++++++++++++++ .../update_rows_avoiding_duplicates_test.php | 71 +++++++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 tests/functions/fixtures/duplicates.xml create mode 100644 tests/functions/update_rows_avoiding_duplicates_test.php diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 65d8be32ad..8608c6ca4f 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -1297,6 +1297,28 @@ function tz_select($default = '', $truncate = false) return $tz_select; } +/** +* Updates rows in given table from a set of values to a new value. +* If this results in rows violating uniqueness constraints, the duplicate +* rows are eliminated. +* +* @param dbal $db Database object +* @param string $table Table on which to perform the update +* @param string $column Column whose values to change +* @param array $from_values An array of values that should be changed +* @param int $to_value The new value +* @return null +*/ +function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_values, $to_value) +{ + $db->sql_return_on_error(true); + $condition = $db->sql_in_set($column, $from_values); + $db->sql_query('UPDATE ' . $table . ' SET ' . $column . ' = ' . (int) $to_value. ' WHERE ' . $condition); + $db->sql_return_on_error(false); + + $db->sql_query('DELETE FROM ' . $table . ' WHERE ' . $condition); +} + // Functions handling topic/post tracking/marking /** @@ -3355,7 +3377,7 @@ function parse_cfg_file($filename, $lines = false) $parsed_items[$key] = $value; } - + if (isset($parsed_items['inherit_from']) && isset($parsed_items['name']) && $parsed_items['inherit_from'] == $parsed_items['name']) { unset($parsed_items['inherit_from']); diff --git a/tests/functions/fixtures/duplicates.xml b/tests/functions/fixtures/duplicates.xml new file mode 100644 index 0000000000..bc08016a8f --- /dev/null +++ b/tests/functions/fixtures/duplicates.xml @@ -0,0 +1,56 @@ + + + + user_id + topic_id + notify_status + + + + 1 + 1 + 1 + + + + + 2 + 2 + 1 + + + 3 + 3 + 1 + + + + + 1 + 4 + 1 + + + 1 + 5 + 1 + + + + + 1 + 6 + 1 + + + 1 + 7 + 1 + + + 2 + 6 + 1 + +
+ diff --git a/tests/functions/update_rows_avoiding_duplicates_test.php b/tests/functions/update_rows_avoiding_duplicates_test.php new file mode 100644 index 0000000000..0e949717d2 --- /dev/null +++ b/tests/functions/update_rows_avoiding_duplicates_test.php @@ -0,0 +1,71 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/duplicates.xml'); + } + + public static function fixture_data() + { + return array( + // description + // from array + // to value + // expected count with to value post update + array( + 'trivial', + array(1), + 10, + 1, + ), + array( + 'no conflict', + array(2), + 3, + 2, + ), + array( + 'conflict', + array(4), + 5, + 1, + ), + array( + 'conflict and no conflict', + array(6), + 7, + 2, + ), + ); + } + + /** + * @dataProvider fixture_data + */ + public function test_trivial_update($description, $from, $to, $expected_result_count) + { + $db = $this->new_dbal(); + + phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $from, $to); + + $sql = 'SELECT count(*) AS count + FROM ' . TOPICS_WATCH_TABLE . ' + WHERE topic_id = ' . $db->sql_escape($to); + $result = $db->sql_query($sql); + $result_count = $db->sql_fetchfield('count'); + $db->sql_freeresult($result); + + $this->assertEquals($expected_result_count, $result_count); + } +} From b0812c43fa05bec8c59e5ff3c7889f0f98089775 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 11 Nov 2012 17:40:58 +0100 Subject: [PATCH 02/22] [ticket/11162] Use integer casting instead of SQL escape. PHPBB3-11162 --- tests/functions/update_rows_avoiding_duplicates_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functions/update_rows_avoiding_duplicates_test.php b/tests/functions/update_rows_avoiding_duplicates_test.php index 0e949717d2..e4e156209d 100644 --- a/tests/functions/update_rows_avoiding_duplicates_test.php +++ b/tests/functions/update_rows_avoiding_duplicates_test.php @@ -61,7 +61,7 @@ class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_cas $sql = 'SELECT count(*) AS count FROM ' . TOPICS_WATCH_TABLE . ' - WHERE topic_id = ' . $db->sql_escape($to); + WHERE topic_id = ' . (int) $to; $result = $db->sql_query($sql); $result_count = $db->sql_fetchfield('count'); $db->sql_freeresult($result); From 7d0cc15b926dda1a53b8151e063e2ffda7441240 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 11 Nov 2012 17:48:59 +0100 Subject: [PATCH 03/22] [ticket/11162] Rename count variable name to remaining_rows. PHPBB3-11162 --- tests/functions/update_rows_avoiding_duplicates_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functions/update_rows_avoiding_duplicates_test.php b/tests/functions/update_rows_avoiding_duplicates_test.php index e4e156209d..0c9ae068a4 100644 --- a/tests/functions/update_rows_avoiding_duplicates_test.php +++ b/tests/functions/update_rows_avoiding_duplicates_test.php @@ -59,11 +59,11 @@ class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_cas phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $from, $to); - $sql = 'SELECT count(*) AS count + $sql = 'SELECT count(*) AS remaining_rows FROM ' . TOPICS_WATCH_TABLE . ' WHERE topic_id = ' . (int) $to; $result = $db->sql_query($sql); - $result_count = $db->sql_fetchfield('count'); + $result_count = $db->sql_fetchfield('remaining_rows'); $db->sql_freeresult($result); $this->assertEquals($expected_result_count, $result_count); From ac9c4d7d59ea458834cb64b9c9020c3de8fe90a2 Mon Sep 17 00:00:00 2001 From: Andreas Fischer Date: Sun, 11 Nov 2012 17:49:21 +0100 Subject: [PATCH 04/22] [ticket/11162] Make count function upper case. PHPBB3-11162 --- tests/functions/update_rows_avoiding_duplicates_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functions/update_rows_avoiding_duplicates_test.php b/tests/functions/update_rows_avoiding_duplicates_test.php index 0c9ae068a4..0d68e22d4a 100644 --- a/tests/functions/update_rows_avoiding_duplicates_test.php +++ b/tests/functions/update_rows_avoiding_duplicates_test.php @@ -59,7 +59,7 @@ class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_cas phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $from, $to); - $sql = 'SELECT count(*) AS remaining_rows + $sql = 'SELECT COUNT(*) AS remaining_rows FROM ' . TOPICS_WATCH_TABLE . ' WHERE topic_id = ' . (int) $to; $result = $db->sql_query($sql); From d0338531cb2a3a386a3894a1b1b081025f53ea05 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 1 Dec 2012 20:12:31 -0500 Subject: [PATCH 05/22] [ticket/11162] An implementation that actually works. PHPBB3-11162 --- phpBB/includes/functions.php | 73 +++++++++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 5 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 8608c6ca4f..13690e79bb 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -1311,12 +1311,75 @@ function tz_select($default = '', $truncate = false) */ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_values, $to_value) { - $db->sql_return_on_error(true); - $condition = $db->sql_in_set($column, $from_values); - $db->sql_query('UPDATE ' . $table . ' SET ' . $column . ' = ' . (int) $to_value. ' WHERE ' . $condition); - $db->sql_return_on_error(false); + $sql = "SELECT $column, user_id + FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $result = $db->sql_query($sql); - $db->sql_query('DELETE FROM ' . $table . ' WHERE ' . $condition); + $old_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $old_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $sql = "SELECT $column, user_id + FROM $table + WHERE $column = '" . (int) $to_value . "'"; + $result = $db->sql_query($sql); + + $new_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $new_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $queries = array(); + $any_found = false; + foreach ($from_values as $from_value) + { + if (!isset($old_user_ids[$from_value])) + { + continue; + } + $any_found = true; + if (empty($new_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value. " + WHERE $column = '" . $db->sql_escape($from_value) . "'"; + $queries[] = $sql; + } + else + { + $different_user_ids = array_diff($old_user_ids[$from_value], $new_user_ids[$to_value]); + if (!empty($different_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value. " + WHERE $column = '" . $db->sql_escape($from_value) . "' + AND " . $db->sql_in_set('user_id', $different_user_ids); + $queries[] = $sql; + } + } + } + + if ($any_found) + { + //$db->sql_transaction('begin'); + + foreach ($queries as $sql) + { + $db->sql_query($sql); + } + + $sql = "DELETE FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $db->sql_query($sql); + + //$db->sql_transaction('commit'); + } } // Functions handling topic/post tracking/marking From 2e947334e563f122c597d7072eddd962453a84ef Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 1 Dec 2012 20:17:01 -0500 Subject: [PATCH 06/22] [ticket/11162] Uncomment transactions. PHPBB3-11162 --- phpBB/includes/functions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 13690e79bb..a24dcfdf5b 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -1367,7 +1367,7 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value if ($any_found) { - //$db->sql_transaction('begin'); + $db->sql_transaction('begin'); foreach ($queries as $sql) { @@ -1378,7 +1378,7 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value WHERE " . $db->sql_in_set($column, $from_values); $db->sql_query($sql); - //$db->sql_transaction('commit'); + $db->sql_transaction('commit'); } } From c2c105df9ff1d8fd48afa1d3abdf23a39584d7ed Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 1 Dec 2012 20:21:29 -0500 Subject: [PATCH 07/22] [ticket/11162] Clarify that only the two tables actually work. PHPBB3-11162 --- phpBB/includes/functions.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index a24dcfdf5b..09487ce10d 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -1302,6 +1302,8 @@ function tz_select($default = '', $truncate = false) * If this results in rows violating uniqueness constraints, the duplicate * rows are eliminated. * +* The only supported tables are bookmarks and topics_watch. +* * @param dbal $db Database object * @param string $table Table on which to perform the update * @param string $column Column whose values to change From 69225bd0a6005e81b7cb58631eb5de7c3d175697 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 1 Dec 2012 20:21:51 -0500 Subject: [PATCH 08/22] [ticket/11162] Use phpbb_update_rows_avoiding_duplicates in mcp. PHPBB3-11162 --- phpBB/includes/mcp/mcp_forum.php | 7 +------ phpBB/includes/mcp/mcp_topic.php | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/phpBB/includes/mcp/mcp_forum.php b/phpBB/includes/mcp/mcp_forum.php index b70601b479..913c55a9d1 100644 --- a/phpBB/includes/mcp/mcp_forum.php +++ b/phpBB/includes/mcp/mcp_forum.php @@ -415,12 +415,7 @@ function merge_topics($forum_id, $topic_ids, $to_topic_id) $success_msg = 'POSTS_MERGED_SUCCESS'; // If the topic no longer exist, we will update the topic watch table. - // To not let it error out on users watching both topics, we just return on an error... - $db->sql_return_on_error(true); - $db->sql_query('UPDATE ' . TOPICS_WATCH_TABLE . ' SET topic_id = ' . (int) $to_topic_id . ' WHERE ' . $db->sql_in_set('topic_id', $topic_ids)); - $db->sql_return_on_error(false); - - $db->sql_query('DELETE FROM ' . TOPICS_WATCH_TABLE . ' WHERE ' . $db->sql_in_set('topic_id', $topic_ids)); + phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); // Link to the new topic $return_link .= (($return_link) ? '

' : '') . sprintf($user->lang['RETURN_NEW_TOPIC'], '', ''); diff --git a/phpBB/includes/mcp/mcp_topic.php b/phpBB/includes/mcp/mcp_topic.php index 7d4edaf362..f40a4bc044 100644 --- a/phpBB/includes/mcp/mcp_topic.php +++ b/phpBB/includes/mcp/mcp_topic.php @@ -620,12 +620,7 @@ function merge_posts($topic_id, $to_topic_id) else { // If the topic no longer exist, we will update the topic watch table. - // To not let it error out on users watching both topics, we just return on an error... - $db->sql_return_on_error(true); - $db->sql_query('UPDATE ' . TOPICS_WATCH_TABLE . ' SET topic_id = ' . (int) $to_topic_id . ' WHERE topic_id = ' . (int) $topic_id); - $db->sql_return_on_error(false); - - $db->sql_query('DELETE FROM ' . TOPICS_WATCH_TABLE . ' WHERE topic_id = ' . (int) $topic_id); + phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); } // Link to the new topic From 05053dacd350c6bacd529df803bdc9876104a278 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 1 Dec 2012 20:24:25 -0500 Subject: [PATCH 09/22] [ticket/11162] Fix inaccurately copy pasted comment. PHPBB3-11162 --- phpBB/includes/mcp/mcp_forum.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/mcp/mcp_forum.php b/phpBB/includes/mcp/mcp_forum.php index 913c55a9d1..e392bef157 100644 --- a/phpBB/includes/mcp/mcp_forum.php +++ b/phpBB/includes/mcp/mcp_forum.php @@ -414,7 +414,7 @@ function merge_topics($forum_id, $topic_ids, $to_topic_id) // Message and return links $success_msg = 'POSTS_MERGED_SUCCESS'; - // If the topic no longer exist, we will update the topic watch table. + // Update the topic watch table. phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); // Link to the new topic From 2fc43e6ed71e4b9abdb76aa179b4e64efaa47ffa Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Sat, 1 Dec 2012 23:11:14 -0500 Subject: [PATCH 10/22] [ticket/11162] No whitespace changes in olympus. PHPBB3-11162 --- phpBB/includes/functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index 09487ce10d..e3feb59d41 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -3442,7 +3442,7 @@ function parse_cfg_file($filename, $lines = false) $parsed_items[$key] = $value; } - + if (isset($parsed_items['inherit_from']) && isset($parsed_items['name']) && $parsed_items['inherit_from'] == $parsed_items['name']) { unset($parsed_items['inherit_from']); From 0f96b1aad378b1fc40620ea57df5ee89224fd5eb Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 4 Dec 2012 23:35:34 -0500 Subject: [PATCH 11/22] [ticket/11162] Move to a separate file to avoid blowing out functions.php. PHPBB3-11162 --- phpBB/includes/functions.php | 87 --------------- phpBB/includes/functions_tricky_update.php | 103 ++++++++++++++++++ .../fixtures/duplicates.xml | 0 .../update_rows_avoiding_duplicates_test.php | 2 +- 4 files changed, 104 insertions(+), 88 deletions(-) create mode 100644 phpBB/includes/functions_tricky_update.php rename tests/{functions => functions_tricky_update}/fixtures/duplicates.xml (100%) rename tests/{functions => functions_tricky_update}/update_rows_avoiding_duplicates_test.php (98%) diff --git a/phpBB/includes/functions.php b/phpBB/includes/functions.php index e3feb59d41..65d8be32ad 100644 --- a/phpBB/includes/functions.php +++ b/phpBB/includes/functions.php @@ -1297,93 +1297,6 @@ function tz_select($default = '', $truncate = false) return $tz_select; } -/** -* Updates rows in given table from a set of values to a new value. -* If this results in rows violating uniqueness constraints, the duplicate -* rows are eliminated. -* -* The only supported tables are bookmarks and topics_watch. -* -* @param dbal $db Database object -* @param string $table Table on which to perform the update -* @param string $column Column whose values to change -* @param array $from_values An array of values that should be changed -* @param int $to_value The new value -* @return null -*/ -function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_values, $to_value) -{ - $sql = "SELECT $column, user_id - FROM $table - WHERE " . $db->sql_in_set($column, $from_values); - $result = $db->sql_query($sql); - - $old_user_ids = array(); - while ($row = $db->sql_fetchrow($result)) - { - $old_user_ids[$row[$column]][] = $row['user_id']; - } - $db->sql_freeresult($result); - - $sql = "SELECT $column, user_id - FROM $table - WHERE $column = '" . (int) $to_value . "'"; - $result = $db->sql_query($sql); - - $new_user_ids = array(); - while ($row = $db->sql_fetchrow($result)) - { - $new_user_ids[$row[$column]][] = $row['user_id']; - } - $db->sql_freeresult($result); - - $queries = array(); - $any_found = false; - foreach ($from_values as $from_value) - { - if (!isset($old_user_ids[$from_value])) - { - continue; - } - $any_found = true; - if (empty($new_user_ids)) - { - $sql = "UPDATE $table - SET $column = " . (int) $to_value. " - WHERE $column = '" . $db->sql_escape($from_value) . "'"; - $queries[] = $sql; - } - else - { - $different_user_ids = array_diff($old_user_ids[$from_value], $new_user_ids[$to_value]); - if (!empty($different_user_ids)) - { - $sql = "UPDATE $table - SET $column = " . (int) $to_value. " - WHERE $column = '" . $db->sql_escape($from_value) . "' - AND " . $db->sql_in_set('user_id', $different_user_ids); - $queries[] = $sql; - } - } - } - - if ($any_found) - { - $db->sql_transaction('begin'); - - foreach ($queries as $sql) - { - $db->sql_query($sql); - } - - $sql = "DELETE FROM $table - WHERE " . $db->sql_in_set($column, $from_values); - $db->sql_query($sql); - - $db->sql_transaction('commit'); - } -} - // Functions handling topic/post tracking/marking /** diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_tricky_update.php new file mode 100644 index 0000000000..a5b9fdacd0 --- /dev/null +++ b/phpBB/includes/functions_tricky_update.php @@ -0,0 +1,103 @@ +sql_in_set($column, $from_values); + $result = $db->sql_query($sql); + + $old_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $old_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $sql = "SELECT $column, user_id + FROM $table + WHERE $column = '" . (int) $to_value . "'"; + $result = $db->sql_query($sql); + + $new_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $new_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $queries = array(); + $any_found = false; + foreach ($from_values as $from_value) + { + if (!isset($old_user_ids[$from_value])) + { + continue; + } + $any_found = true; + if (empty($new_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value. " + WHERE $column = '" . $db->sql_escape($from_value) . "'"; + $queries[] = $sql; + } + else + { + $different_user_ids = array_diff($old_user_ids[$from_value], $new_user_ids[$to_value]); + if (!empty($different_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value. " + WHERE $column = '" . $db->sql_escape($from_value) . "' + AND " . $db->sql_in_set('user_id', $different_user_ids); + $queries[] = $sql; + } + } + } + + if ($any_found) + { + $db->sql_transaction('begin'); + + foreach ($queries as $sql) + { + $db->sql_query($sql); + } + + $sql = "DELETE FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $db->sql_query($sql); + + $db->sql_transaction('commit'); + } +} diff --git a/tests/functions/fixtures/duplicates.xml b/tests/functions_tricky_update/fixtures/duplicates.xml similarity index 100% rename from tests/functions/fixtures/duplicates.xml rename to tests/functions_tricky_update/fixtures/duplicates.xml diff --git a/tests/functions/update_rows_avoiding_duplicates_test.php b/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php similarity index 98% rename from tests/functions/update_rows_avoiding_duplicates_test.php rename to tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php index 0d68e22d4a..6940122393 100644 --- a/tests/functions/update_rows_avoiding_duplicates_test.php +++ b/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php @@ -7,7 +7,7 @@ * */ -require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; +require_once dirname(__FILE__) . '/../../phpBB/includes/functions_tricky_update.php'; class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_case { From abca64b1dfbe11f73675a3e5457fb9f0038f1cb6 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 4 Dec 2012 23:37:14 -0500 Subject: [PATCH 12/22] [ticket/11162] Add includes. PHPBB3-11162 --- phpBB/includes/mcp/mcp_forum.php | 4 ++++ phpBB/includes/mcp/mcp_topic.php | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/phpBB/includes/mcp/mcp_forum.php b/phpBB/includes/mcp/mcp_forum.php index e392bef157..3603224735 100644 --- a/phpBB/includes/mcp/mcp_forum.php +++ b/phpBB/includes/mcp/mcp_forum.php @@ -415,6 +415,10 @@ function merge_topics($forum_id, $topic_ids, $to_topic_id) $success_msg = 'POSTS_MERGED_SUCCESS'; // Update the topic watch table. + if (!function_exists('phpbb_update_rows_avoiding_duplicates')) + { + include($phpbb_root_path . 'includes/functions_tricky_update.' . $phpEx); + } phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); // Link to the new topic diff --git a/phpBB/includes/mcp/mcp_topic.php b/phpBB/includes/mcp/mcp_topic.php index f40a4bc044..1fb3cb9d73 100644 --- a/phpBB/includes/mcp/mcp_topic.php +++ b/phpBB/includes/mcp/mcp_topic.php @@ -620,6 +620,10 @@ function merge_posts($topic_id, $to_topic_id) else { // If the topic no longer exist, we will update the topic watch table. + if (!function_exists('phpbb_update_rows_avoiding_duplicates')) + { + include($phpbb_root_path . 'includes/functions_tricky_update.' . $phpEx); + } phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); } From 58951ef1057bd5f0d1098f9536eb8a84c532833c Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 4 Dec 2012 23:37:57 -0500 Subject: [PATCH 13/22] [ticket/11162] The test is not at all trivial. PHPBB3-11162 --- .../update_rows_avoiding_duplicates_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php b/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php index 6940122393..4849605e9c 100644 --- a/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php +++ b/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php @@ -53,7 +53,7 @@ class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_cas /** * @dataProvider fixture_data */ - public function test_trivial_update($description, $from, $to, $expected_result_count) + public function test_update($description, $from, $to, $expected_result_count) { $db = $this->new_dbal(); From efe122b03200155479920890defc2a3dc689bdd7 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Tue, 4 Dec 2012 23:40:30 -0500 Subject: [PATCH 14/22] [ticket/11162] This test really only works for bookmarks. PHPBB3-11162 --- .../fixtures/bookmarks_duplicates.xml | 47 +++++++++++++++++++ ...icates.xml => topics_watch_duplicates.xml} | 0 .../update_rows_avoiding_duplicates_test.php | 6 +-- 3 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 tests/functions_tricky_update/fixtures/bookmarks_duplicates.xml rename tests/functions_tricky_update/fixtures/{duplicates.xml => topics_watch_duplicates.xml} (100%) diff --git a/tests/functions_tricky_update/fixtures/bookmarks_duplicates.xml b/tests/functions_tricky_update/fixtures/bookmarks_duplicates.xml new file mode 100644 index 0000000000..d49f76b073 --- /dev/null +++ b/tests/functions_tricky_update/fixtures/bookmarks_duplicates.xml @@ -0,0 +1,47 @@ + + + + user_id + topic_id + + + + 1 + 1 + + + + + 2 + 2 + + + 3 + 3 + + + + + 1 + 4 + + + 1 + 5 + + + + + 1 + 6 + + + 1 + 7 + + + 2 + 6 + +
+ diff --git a/tests/functions_tricky_update/fixtures/duplicates.xml b/tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml similarity index 100% rename from tests/functions_tricky_update/fixtures/duplicates.xml rename to tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml diff --git a/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php b/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php index 4849605e9c..6142997408 100644 --- a/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php +++ b/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php @@ -13,7 +13,7 @@ class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_cas { public function getDataSet() { - return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/duplicates.xml'); + return $this->createXMLDataSet(dirname(__FILE__).'/fixtures/bookmarks_duplicates.xml'); } public static function fixture_data() @@ -57,10 +57,10 @@ class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_cas { $db = $this->new_dbal(); - phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $from, $to); + phpbb_update_rows_avoiding_duplicates($db, BOOKMARKS_TABLE, 'topic_id', $from, $to); $sql = 'SELECT COUNT(*) AS remaining_rows - FROM ' . TOPICS_WATCH_TABLE . ' + FROM ' . BOOKMARKS_TABLE . ' WHERE topic_id = ' . (int) $to; $result = $db->sql_query($sql); $result_count = $db->sql_fetchfield('remaining_rows'); From 6872104aa95a9f2aad565189e25bbf54abc3ca2c Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 5 Dec 2012 00:07:01 -0500 Subject: [PATCH 15/22] [ticket/11162] Account for notify_status. PHPBB3-11162 --- phpBB/includes/functions_tricky_update.php | 111 +++++++++++++++++- .../fixtures/topics_watch_duplicates.xml | 38 ++++-- ...avoiding_duplicates_notify_status_test.php | 100 ++++++++++++++++ 3 files changed, 239 insertions(+), 10 deletions(-) create mode 100644 tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_tricky_update.php index a5b9fdacd0..cf922d7014 100644 --- a/phpBB/includes/functions_tricky_update.php +++ b/phpBB/includes/functions_tricky_update.php @@ -20,7 +20,7 @@ if (!defined('IN_PHPBB')) * If this results in rows violating uniqueness constraints, the duplicate * rows are eliminated. * -* The only supported tables are bookmarks and topics_watch. +* The only supported table is bookmarks. * * @param dbal $db Database object * @param string $table Table on which to perform the update @@ -67,7 +67,7 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value if (empty($new_user_ids)) { $sql = "UPDATE $table - SET $column = " . (int) $to_value. " + SET $column = " . (int) $to_value . " WHERE $column = '" . $db->sql_escape($from_value) . "'"; $queries[] = $sql; } @@ -77,7 +77,7 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value if (!empty($different_user_ids)) { $sql = "UPDATE $table - SET $column = " . (int) $to_value. " + SET $column = " . (int) $to_value . " WHERE $column = '" . $db->sql_escape($from_value) . "' AND " . $db->sql_in_set('user_id', $different_user_ids); $queries[] = $sql; @@ -101,3 +101,108 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value $db->sql_transaction('commit'); } } + +/** +* Updates rows in given table from a set of values to a new value. +* If this results in rows violating uniqueness constraints, the duplicate +* rows are merged respecting notify_status (0 takes precedence over 1). +* +* The only supported table is topics_watch. +* +* @param dbal $db Database object +* @param string $table Table on which to perform the update +* @param string $column Column whose values to change +* @param array $from_values An array of values that should be changed +* @param int $to_value The new value +* @return null +*/ +function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $column, $from_values, $to_value) +{ + $sql = "SELECT $column, user_id, notify_status + FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $result = $db->sql_query($sql); + + $old_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $old_user_ids[(int) $row['notify_status']][$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $sql = "SELECT $column, user_id + FROM $table + WHERE $column = '" . (int) $to_value . "'"; + $result = $db->sql_query($sql); + + $new_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $new_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $queries = array(); + $any_found = false; + $extra_updates = array( + 0 => 'notify_status = 0', + 1 => '', + ); + foreach ($from_values as $from_value) + { + foreach ($extra_updates as $notify_status => $extra_update) { + if (!isset($old_user_ids[$notify_status][$from_value])) + { + continue; + } + $any_found = true; + if (empty($new_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value . " + WHERE $column = '" . $db->sql_escape($from_value) . "'"; + $queries[] = $sql; + } + else + { + $different_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $new_user_ids[$to_value]); + if (!empty($different_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value . " + WHERE $column = '" . $db->sql_escape($from_value) . "' + AND " . $db->sql_in_set('user_id', $different_user_ids); + $queries[] = $sql; + } + + if ($extra_update) { + $same_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $different_user_ids); + if (!empty($same_user_ids)) + { + $sql = "UPDATE $table + SET $extra_update + WHERE $column = '" . (int) $to_value . "' + AND " . $db->sql_in_set('user_id', $same_user_ids); + $queries[] = $sql; + } + } + } + } + } + + if ($any_found) + { + $db->sql_transaction('begin'); + + foreach ($queries as $sql) + { + $db->sql_query($sql); + } + + $sql = "DELETE FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $db->sql_query($sql); + + $db->sql_transaction('commit'); + } +} diff --git a/tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml b/tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml index bc08016a8f..c387bb737a 100644 --- a/tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml +++ b/tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml @@ -14,17 +14,17 @@ - 2 + 1 2 1 + 2 3 - 3 - 1 + 0 - + 1 4 @@ -36,20 +36,44 @@ 1 - + 1 6 - 1 + 0 1 7 1 + + + + 1 + 8 + 1 + + + 1 + 9 + 0 + + + + + 1 + 10 + 0 + + + 1 + 11 + 1 + 2 - 6 + 10 1 diff --git a/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php b/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php new file mode 100644 index 0000000000..aa739c5f04 --- /dev/null +++ b/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php @@ -0,0 +1,100 @@ +createXMLDataSet(dirname(__FILE__).'/fixtures/topics_watch_duplicates.xml'); + } + + public static function fixture_data() + { + return array( + // description + // from array + // to value + // expected count with to value post update + // expected notify_status values + array( + 'trivial', + array(1), + 1000, + 1, + 1, + ), + array( + 'no conflict', + array(2), + 3, + 2, + 1, + ), + array( + 'conflict, same notify status', + array(4), + 5, + 1, + 1, + ), + array( + 'conflict, notify status 0 into 1', + array(6), + 7, + 1, + 0, + ), + array( + 'conflict, notify status 1 into 0', + array(8), + 9, + 1, + 0, + ), + array( + 'conflict and no conflict', + array(10), + 11, + 2, + 0, + ), + ); + } + + /** + * @dataProvider fixture_data + */ + public function test_update($description, $from, $to, $expected_result_count, $expected_notify_status) + { + $db = $this->new_dbal(); + + phpbb_update_rows_avoiding_duplicates_notify_status($db, TOPICS_WATCH_TABLE, 'topic_id', $from, $to); + + $sql = 'SELECT COUNT(*) AS remaining_rows + FROM ' . TOPICS_WATCH_TABLE . ' + WHERE topic_id = ' . (int) $to; + $result = $db->sql_query($sql); + $result_count = $db->sql_fetchfield('remaining_rows'); + $db->sql_freeresult($result); + + $this->assertEquals($expected_result_count, $result_count); + + // user id of 1 is the user being updated + $sql = 'SELECT notify_status + FROM ' . TOPICS_WATCH_TABLE . ' + WHERE topic_id = ' . (int) $to . ' AND user_id = 1'; + $result = $db->sql_query($sql); + $notify_status = $db->sql_fetchfield('notify_status'); + $db->sql_freeresult($result); + + $this->assertEquals($expected_notify_status, $notify_status); + } +} From 1a411c5658da769e28b03332ed618d5e701dab79 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 5 Dec 2012 00:10:02 -0500 Subject: [PATCH 16/22] [ticket/11162] Use correct functions. PHPBB3-11162 --- phpBB/includes/mcp/mcp_forum.php | 4 ++-- phpBB/includes/mcp/mcp_topic.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/mcp/mcp_forum.php b/phpBB/includes/mcp/mcp_forum.php index 3603224735..cd938e83a0 100644 --- a/phpBB/includes/mcp/mcp_forum.php +++ b/phpBB/includes/mcp/mcp_forum.php @@ -415,11 +415,11 @@ function merge_topics($forum_id, $topic_ids, $to_topic_id) $success_msg = 'POSTS_MERGED_SUCCESS'; // Update the topic watch table. - if (!function_exists('phpbb_update_rows_avoiding_duplicates')) + if (!function_exists('phpbb_update_rows_avoiding_duplicates_notify_status')) { include($phpbb_root_path . 'includes/functions_tricky_update.' . $phpEx); } - phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); + phpbb_update_rows_avoiding_duplicates_notify_status($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); // Link to the new topic $return_link .= (($return_link) ? '

' : '') . sprintf($user->lang['RETURN_NEW_TOPIC'], '', ''); diff --git a/phpBB/includes/mcp/mcp_topic.php b/phpBB/includes/mcp/mcp_topic.php index 1fb3cb9d73..29dabdd2a2 100644 --- a/phpBB/includes/mcp/mcp_topic.php +++ b/phpBB/includes/mcp/mcp_topic.php @@ -620,11 +620,11 @@ function merge_posts($topic_id, $to_topic_id) else { // If the topic no longer exist, we will update the topic watch table. - if (!function_exists('phpbb_update_rows_avoiding_duplicates')) + if (!function_exists('phpbb_update_rows_avoiding_duplicates_notify_status')) { include($phpbb_root_path . 'includes/functions_tricky_update.' . $phpEx); } - phpbb_update_rows_avoiding_duplicates($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); + phpbb_update_rows_avoiding_duplicates_notify_status($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); } // Link to the new topic From 94ebc5707836d931e24e77f5ce1b3a16d0410fb8 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 5 Dec 2012 10:40:42 -0500 Subject: [PATCH 17/22] [ticket/11162] Newlines to LF. PHPBB3-11162 --- phpBB/includes/functions_tricky_update.php | 416 ++++++++++----------- 1 file changed, 208 insertions(+), 208 deletions(-) diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_tricky_update.php index cf922d7014..c8fcb9b6f0 100644 --- a/phpBB/includes/functions_tricky_update.php +++ b/phpBB/includes/functions_tricky_update.php @@ -1,208 +1,208 @@ -sql_in_set($column, $from_values); - $result = $db->sql_query($sql); - - $old_user_ids = array(); - while ($row = $db->sql_fetchrow($result)) - { - $old_user_ids[$row[$column]][] = $row['user_id']; - } - $db->sql_freeresult($result); - - $sql = "SELECT $column, user_id - FROM $table - WHERE $column = '" . (int) $to_value . "'"; - $result = $db->sql_query($sql); - - $new_user_ids = array(); - while ($row = $db->sql_fetchrow($result)) - { - $new_user_ids[$row[$column]][] = $row['user_id']; - } - $db->sql_freeresult($result); - - $queries = array(); - $any_found = false; - foreach ($from_values as $from_value) - { - if (!isset($old_user_ids[$from_value])) - { - continue; - } - $any_found = true; - if (empty($new_user_ids)) - { - $sql = "UPDATE $table - SET $column = " . (int) $to_value . " - WHERE $column = '" . $db->sql_escape($from_value) . "'"; - $queries[] = $sql; - } - else - { - $different_user_ids = array_diff($old_user_ids[$from_value], $new_user_ids[$to_value]); - if (!empty($different_user_ids)) - { - $sql = "UPDATE $table - SET $column = " . (int) $to_value . " - WHERE $column = '" . $db->sql_escape($from_value) . "' - AND " . $db->sql_in_set('user_id', $different_user_ids); - $queries[] = $sql; - } - } - } - - if ($any_found) - { - $db->sql_transaction('begin'); - - foreach ($queries as $sql) - { - $db->sql_query($sql); - } - - $sql = "DELETE FROM $table - WHERE " . $db->sql_in_set($column, $from_values); - $db->sql_query($sql); - - $db->sql_transaction('commit'); - } -} - -/** -* Updates rows in given table from a set of values to a new value. -* If this results in rows violating uniqueness constraints, the duplicate -* rows are merged respecting notify_status (0 takes precedence over 1). -* -* The only supported table is topics_watch. -* -* @param dbal $db Database object -* @param string $table Table on which to perform the update -* @param string $column Column whose values to change -* @param array $from_values An array of values that should be changed -* @param int $to_value The new value -* @return null -*/ -function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $column, $from_values, $to_value) -{ - $sql = "SELECT $column, user_id, notify_status - FROM $table - WHERE " . $db->sql_in_set($column, $from_values); - $result = $db->sql_query($sql); - - $old_user_ids = array(); - while ($row = $db->sql_fetchrow($result)) - { - $old_user_ids[(int) $row['notify_status']][$row[$column]][] = $row['user_id']; - } - $db->sql_freeresult($result); - - $sql = "SELECT $column, user_id - FROM $table - WHERE $column = '" . (int) $to_value . "'"; - $result = $db->sql_query($sql); - - $new_user_ids = array(); - while ($row = $db->sql_fetchrow($result)) - { - $new_user_ids[$row[$column]][] = $row['user_id']; - } - $db->sql_freeresult($result); - - $queries = array(); - $any_found = false; - $extra_updates = array( - 0 => 'notify_status = 0', - 1 => '', - ); - foreach ($from_values as $from_value) - { - foreach ($extra_updates as $notify_status => $extra_update) { - if (!isset($old_user_ids[$notify_status][$from_value])) - { - continue; - } - $any_found = true; - if (empty($new_user_ids)) - { - $sql = "UPDATE $table - SET $column = " . (int) $to_value . " - WHERE $column = '" . $db->sql_escape($from_value) . "'"; - $queries[] = $sql; - } - else - { - $different_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $new_user_ids[$to_value]); - if (!empty($different_user_ids)) - { - $sql = "UPDATE $table - SET $column = " . (int) $to_value . " - WHERE $column = '" . $db->sql_escape($from_value) . "' - AND " . $db->sql_in_set('user_id', $different_user_ids); - $queries[] = $sql; - } - - if ($extra_update) { - $same_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $different_user_ids); - if (!empty($same_user_ids)) - { - $sql = "UPDATE $table - SET $extra_update - WHERE $column = '" . (int) $to_value . "' - AND " . $db->sql_in_set('user_id', $same_user_ids); - $queries[] = $sql; - } - } - } - } - } - - if ($any_found) - { - $db->sql_transaction('begin'); - - foreach ($queries as $sql) - { - $db->sql_query($sql); - } - - $sql = "DELETE FROM $table - WHERE " . $db->sql_in_set($column, $from_values); - $db->sql_query($sql); - - $db->sql_transaction('commit'); - } -} +sql_in_set($column, $from_values); + $result = $db->sql_query($sql); + + $old_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $old_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $sql = "SELECT $column, user_id + FROM $table + WHERE $column = '" . (int) $to_value . "'"; + $result = $db->sql_query($sql); + + $new_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $new_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $queries = array(); + $any_found = false; + foreach ($from_values as $from_value) + { + if (!isset($old_user_ids[$from_value])) + { + continue; + } + $any_found = true; + if (empty($new_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value . " + WHERE $column = '" . $db->sql_escape($from_value) . "'"; + $queries[] = $sql; + } + else + { + $different_user_ids = array_diff($old_user_ids[$from_value], $new_user_ids[$to_value]); + if (!empty($different_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value . " + WHERE $column = '" . $db->sql_escape($from_value) . "' + AND " . $db->sql_in_set('user_id', $different_user_ids); + $queries[] = $sql; + } + } + } + + if ($any_found) + { + $db->sql_transaction('begin'); + + foreach ($queries as $sql) + { + $db->sql_query($sql); + } + + $sql = "DELETE FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $db->sql_query($sql); + + $db->sql_transaction('commit'); + } +} + +/** +* Updates rows in given table from a set of values to a new value. +* If this results in rows violating uniqueness constraints, the duplicate +* rows are merged respecting notify_status (0 takes precedence over 1). +* +* The only supported table is topics_watch. +* +* @param dbal $db Database object +* @param string $table Table on which to perform the update +* @param string $column Column whose values to change +* @param array $from_values An array of values that should be changed +* @param int $to_value The new value +* @return null +*/ +function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $column, $from_values, $to_value) +{ + $sql = "SELECT $column, user_id, notify_status + FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $result = $db->sql_query($sql); + + $old_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $old_user_ids[(int) $row['notify_status']][$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $sql = "SELECT $column, user_id + FROM $table + WHERE $column = '" . (int) $to_value . "'"; + $result = $db->sql_query($sql); + + $new_user_ids = array(); + while ($row = $db->sql_fetchrow($result)) + { + $new_user_ids[$row[$column]][] = $row['user_id']; + } + $db->sql_freeresult($result); + + $queries = array(); + $any_found = false; + $extra_updates = array( + 0 => 'notify_status = 0', + 1 => '', + ); + foreach ($from_values as $from_value) + { + foreach ($extra_updates as $notify_status => $extra_update) { + if (!isset($old_user_ids[$notify_status][$from_value])) + { + continue; + } + $any_found = true; + if (empty($new_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value . " + WHERE $column = '" . $db->sql_escape($from_value) . "'"; + $queries[] = $sql; + } + else + { + $different_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $new_user_ids[$to_value]); + if (!empty($different_user_ids)) + { + $sql = "UPDATE $table + SET $column = " . (int) $to_value . " + WHERE $column = '" . $db->sql_escape($from_value) . "' + AND " . $db->sql_in_set('user_id', $different_user_ids); + $queries[] = $sql; + } + + if ($extra_update) { + $same_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $different_user_ids); + if (!empty($same_user_ids)) + { + $sql = "UPDATE $table + SET $extra_update + WHERE $column = '" . (int) $to_value . "' + AND " . $db->sql_in_set('user_id', $same_user_ids); + $queries[] = $sql; + } + } + } + } + } + + if ($any_found) + { + $db->sql_transaction('begin'); + + foreach ($queries as $sql) + { + $db->sql_query($sql); + } + + $sql = "DELETE FROM $table + WHERE " . $db->sql_in_set($column, $from_values); + $db->sql_query($sql); + + $db->sql_transaction('commit'); + } +} From 16966f52d37e4ebdca4f3846c81dfa85b193501e Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 5 Dec 2012 10:41:08 -0500 Subject: [PATCH 18/22] [ticket/11162] Reformat. PHPBB3-11162 --- phpBB/includes/functions_tricky_update.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_tricky_update.php index c8fcb9b6f0..10321618b2 100644 --- a/phpBB/includes/functions_tricky_update.php +++ b/phpBB/includes/functions_tricky_update.php @@ -150,7 +150,8 @@ function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $colum ); foreach ($from_values as $from_value) { - foreach ($extra_updates as $notify_status => $extra_update) { + foreach ($extra_updates as $notify_status => $extra_update) + { if (!isset($old_user_ids[$notify_status][$from_value])) { continue; @@ -174,8 +175,9 @@ function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $colum AND " . $db->sql_in_set('user_id', $different_user_ids); $queries[] = $sql; } - - if ($extra_update) { + + if ($extra_update) + { $same_user_ids = array_diff($old_user_ids[$notify_status][$from_value], $different_user_ids); if (!empty($same_user_ids)) { From fe87d441eeb54bf5efb56a0f69f42848d9ef53d5 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 5 Dec 2012 10:44:36 -0500 Subject: [PATCH 19/22] [ticket/11162] Review comments fixed. PHPBB3-11162 --- phpBB/includes/functions_tricky_update.php | 12 ++++++------ ...e_rows_avoiding_duplicates_notify_status_test.php | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_tricky_update.php index 10321618b2..05cb65e68d 100644 --- a/phpBB/includes/functions_tricky_update.php +++ b/phpBB/includes/functions_tricky_update.php @@ -39,19 +39,19 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value $old_user_ids = array(); while ($row = $db->sql_fetchrow($result)) { - $old_user_ids[$row[$column]][] = $row['user_id']; + $old_user_ids[$row[$column]][] = (int) $row['user_id']; } $db->sql_freeresult($result); $sql = "SELECT $column, user_id FROM $table - WHERE $column = '" . (int) $to_value . "'"; + WHERE $column = " . (int) $to_value; $result = $db->sql_query($sql); $new_user_ids = array(); while ($row = $db->sql_fetchrow($result)) { - $new_user_ids[$row[$column]][] = $row['user_id']; + $new_user_ids[$row[$column]][] = (int) $row['user_id']; } $db->sql_freeresult($result); @@ -126,19 +126,19 @@ function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $colum $old_user_ids = array(); while ($row = $db->sql_fetchrow($result)) { - $old_user_ids[(int) $row['notify_status']][$row[$column]][] = $row['user_id']; + $old_user_ids[(int) $row['notify_status']][$row[$column]][] = (int) $row['user_id']; } $db->sql_freeresult($result); $sql = "SELECT $column, user_id FROM $table - WHERE $column = '" . (int) $to_value . "'"; + WHERE $column = " . (int) $to_value; $result = $db->sql_query($sql); $new_user_ids = array(); while ($row = $db->sql_fetchrow($result)) { - $new_user_ids[$row[$column]][] = $row['user_id']; + $new_user_ids[$row[$column]][] = (int) $row['user_id']; } $db->sql_freeresult($result); diff --git a/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php b/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php index aa739c5f04..9052585552 100644 --- a/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php +++ b/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php @@ -90,7 +90,8 @@ class phpbb_update_rows_avoiding_duplicates_notify_status_test extends phpbb_dat // user id of 1 is the user being updated $sql = 'SELECT notify_status FROM ' . TOPICS_WATCH_TABLE . ' - WHERE topic_id = ' . (int) $to . ' AND user_id = 1'; + WHERE topic_id = ' . (int) $to . ' + AND user_id = 1'; $result = $db->sql_query($sql); $notify_status = $db->sql_fetchfield('notify_status'); $db->sql_freeresult($result); From f5de11438c471a76fc5c5f3a4b8c4c29d07ed734 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 5 Dec 2012 10:47:26 -0500 Subject: [PATCH 20/22] [ticket/11162] Use empty($queries). PHPBB3-11162 --- phpBB/includes/functions_tricky_update.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_tricky_update.php index 05cb65e68d..664c246888 100644 --- a/phpBB/includes/functions_tricky_update.php +++ b/phpBB/includes/functions_tricky_update.php @@ -56,14 +56,12 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value $db->sql_freeresult($result); $queries = array(); - $any_found = false; foreach ($from_values as $from_value) { if (!isset($old_user_ids[$from_value])) { continue; } - $any_found = true; if (empty($new_user_ids)) { $sql = "UPDATE $table @@ -85,7 +83,7 @@ function phpbb_update_rows_avoiding_duplicates($db, $table, $column, $from_value } } - if ($any_found) + if (!empty($queries)) { $db->sql_transaction('begin'); @@ -143,7 +141,6 @@ function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $colum $db->sql_freeresult($result); $queries = array(); - $any_found = false; $extra_updates = array( 0 => 'notify_status = 0', 1 => '', @@ -156,7 +153,6 @@ function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $colum { continue; } - $any_found = true; if (empty($new_user_ids)) { $sql = "UPDATE $table @@ -192,7 +188,7 @@ function phpbb_update_rows_avoiding_duplicates_notify_status($db, $table, $colum } } - if ($any_found) + if (!empty($queries)) { $db->sql_transaction('begin'); From e2c67a8e42beefe1631a90feeb2215e2137c944b Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Wed, 12 Dec 2012 21:46:38 -0500 Subject: [PATCH 21/22] [ticket/11162] Rename tricky updates to database helper. PHPBB3-11162 --- ...unctions_tricky_update.php => functions_database_helper.php} | 0 phpBB/includes/mcp/mcp_forum.php | 2 +- phpBB/includes/mcp/mcp_topic.php | 2 +- .../fixtures/bookmarks_duplicates.xml | 0 .../fixtures/topics_watch_duplicates.xml | 0 .../update_rows_avoiding_duplicates_notify_status_test.php | 2 +- .../update_rows_avoiding_duplicates_test.php | 2 +- 7 files changed, 4 insertions(+), 4 deletions(-) rename phpBB/includes/{functions_tricky_update.php => functions_database_helper.php} (100%) rename tests/{functions_tricky_update => functions_database_helper}/fixtures/bookmarks_duplicates.xml (100%) rename tests/{functions_tricky_update => functions_database_helper}/fixtures/topics_watch_duplicates.xml (100%) rename tests/{functions_tricky_update => functions_database_helper}/update_rows_avoiding_duplicates_notify_status_test.php (98%) rename tests/{functions_tricky_update => functions_database_helper}/update_rows_avoiding_duplicates_test.php (98%) diff --git a/phpBB/includes/functions_tricky_update.php b/phpBB/includes/functions_database_helper.php similarity index 100% rename from phpBB/includes/functions_tricky_update.php rename to phpBB/includes/functions_database_helper.php diff --git a/phpBB/includes/mcp/mcp_forum.php b/phpBB/includes/mcp/mcp_forum.php index cd938e83a0..db9fbd90bd 100644 --- a/phpBB/includes/mcp/mcp_forum.php +++ b/phpBB/includes/mcp/mcp_forum.php @@ -417,7 +417,7 @@ function merge_topics($forum_id, $topic_ids, $to_topic_id) // Update the topic watch table. if (!function_exists('phpbb_update_rows_avoiding_duplicates_notify_status')) { - include($phpbb_root_path . 'includes/functions_tricky_update.' . $phpEx); + include($phpbb_root_path . 'includes/functions_database_helper.' . $phpEx); } phpbb_update_rows_avoiding_duplicates_notify_status($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); diff --git a/phpBB/includes/mcp/mcp_topic.php b/phpBB/includes/mcp/mcp_topic.php index 29dabdd2a2..66d0c7a47e 100644 --- a/phpBB/includes/mcp/mcp_topic.php +++ b/phpBB/includes/mcp/mcp_topic.php @@ -622,7 +622,7 @@ function merge_posts($topic_id, $to_topic_id) // If the topic no longer exist, we will update the topic watch table. if (!function_exists('phpbb_update_rows_avoiding_duplicates_notify_status')) { - include($phpbb_root_path . 'includes/functions_tricky_update.' . $phpEx); + include($phpbb_root_path . 'includes/functions_database_helper.' . $phpEx); } phpbb_update_rows_avoiding_duplicates_notify_status($db, TOPICS_WATCH_TABLE, 'topic_id', $topic_ids, $to_topic_id); } diff --git a/tests/functions_tricky_update/fixtures/bookmarks_duplicates.xml b/tests/functions_database_helper/fixtures/bookmarks_duplicates.xml similarity index 100% rename from tests/functions_tricky_update/fixtures/bookmarks_duplicates.xml rename to tests/functions_database_helper/fixtures/bookmarks_duplicates.xml diff --git a/tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml b/tests/functions_database_helper/fixtures/topics_watch_duplicates.xml similarity index 100% rename from tests/functions_tricky_update/fixtures/topics_watch_duplicates.xml rename to tests/functions_database_helper/fixtures/topics_watch_duplicates.xml diff --git a/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php b/tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php similarity index 98% rename from tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php rename to tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php index 9052585552..6c0f2da1e1 100644 --- a/tests/functions_tricky_update/update_rows_avoiding_duplicates_notify_status_test.php +++ b/tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php @@ -7,7 +7,7 @@ * */ -require_once dirname(__FILE__) . '/../../phpBB/includes/functions_tricky_update.php'; +require_once dirname(__FILE__) . '/../../phpBB/includes/functions_database_helper.php'; class phpbb_update_rows_avoiding_duplicates_notify_status_test extends phpbb_database_test_case { diff --git a/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php b/tests/functions_database_helper/update_rows_avoiding_duplicates_test.php similarity index 98% rename from tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php rename to tests/functions_database_helper/update_rows_avoiding_duplicates_test.php index 6142997408..2f01d29d15 100644 --- a/tests/functions_tricky_update/update_rows_avoiding_duplicates_test.php +++ b/tests/functions_database_helper/update_rows_avoiding_duplicates_test.php @@ -7,7 +7,7 @@ * */ -require_once dirname(__FILE__) . '/../../phpBB/includes/functions_tricky_update.php'; +require_once dirname(__FILE__) . '/../../phpBB/includes/functions_database_helper.php'; class phpbb_update_rows_avoiding_duplicates_test extends phpbb_database_test_case { From a686910958c0ca6eb64c7be10d60d1ebe15999c9 Mon Sep 17 00:00:00 2001 From: Oleg Pudeyev Date: Thu, 13 Dec 2012 03:07:25 -0500 Subject: [PATCH 22/22] [ticket/11162] Reformat. PHPBB3-11162 --- .../update_rows_avoiding_duplicates_notify_status_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php b/tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php index 6c0f2da1e1..d4881daf7e 100644 --- a/tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php +++ b/tests/functions_database_helper/update_rows_avoiding_duplicates_notify_status_test.php @@ -91,7 +91,7 @@ class phpbb_update_rows_avoiding_duplicates_notify_status_test extends phpbb_dat $sql = 'SELECT notify_status FROM ' . TOPICS_WATCH_TABLE . ' WHERE topic_id = ' . (int) $to . ' - AND user_id = 1'; + AND user_id = 1'; $result = $db->sql_query($sql); $notify_status = $db->sql_fetchfield('notify_status'); $db->sql_freeresult($result);