From e7015bf1dd563dbe96316411e6acfce8d2cac9a1 Mon Sep 17 00:00:00 2001 From: Oliver Schramm Date: Mon, 1 Oct 2018 23:40:15 +0200 Subject: [PATCH] [ticket/9687] Fix code style and tests PHPBB3-9687 --- phpBB/includes/functions_user.php | 2 +- phpBB/phpbb/ban/manager.php | 3 +- phpBB/phpbb/ban/type/base.php | 2 +- phpBB/phpbb/ban/type/email.php | 1 - phpBB/phpbb/ban/type/type_interface.php | 2 - .../db/migration/data/v330/ban_table_p1.php | 2 +- .../db/migration/data/v330/ban_table_p2.php | 2 +- tests/dbal/boolean_processor_test.php | 9 ++-- tests/dbal/cross_join_test.php | 4 +- tests/dbal/fixtures/boolean_processor.xml | 6 ++- tests/dbal/fixtures/massmail_crossjoin.xml | 6 ++- tests/functions/fixtures/banned_users.xml | 2 +- tests/functions_user/delete_user_test.php | 2 +- tests/functions_user/fixtures/delete_user.xml | 42 +++++++++++-------- tests/session/check_ban_test.php | 4 +- tests/session/fixtures/sessions_banlist.xml | 34 ++++++++++----- tests/session/fixtures/sessions_empty.xml | 10 +++-- 17 files changed, 78 insertions(+), 55 deletions(-) diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index b910078f7b..84b3b30e1f 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -749,7 +749,7 @@ function user_delete($mode, $user_ids, $retain_username = true) $db->sql_query($sql); // Delete the user_id from the banlist - $sql = 'DELETE FROM ' . BAN_TABLE . ' + $sql = 'DELETE FROM ' . BANS_TABLE . ' WHERE ban_mode = \'user\' AND ' . $db->sql_in_set('ban_item', $user_ids); $db->sql_query($sql); diff --git a/phpBB/phpbb/ban/manager.php b/phpBB/phpbb/ban/manager.php index 5f0e565148..346387349e 100644 --- a/phpBB/phpbb/ban/manager.php +++ b/phpBB/phpbb/ban/manager.php @@ -15,7 +15,6 @@ namespace phpbb\ban; use phpbb\ban\exception\ban_insert_failed_exception; use phpbb\ban\exception\invalid_length_exception; -use phpbb\ban\exception\no_items_specified_exception; use phpbb\ban\exception\type_not_found_exception; class manager @@ -189,7 +188,7 @@ class manager $user_ids = []; while ($row = $this->db->sql_fetchrow($result)) { - $user_ids[] = (int)$row['user_id']; + $user_ids[] = (int) $row['user_id']; } $this->db->sql_freeresult($result); } diff --git a/phpBB/phpbb/ban/type/base.php b/phpBB/phpbb/ban/type/base.php index 9fbe2c392b..9a1e70ab77 100644 --- a/phpBB/phpbb/ban/type/base.php +++ b/phpBB/phpbb/ban/type/base.php @@ -95,7 +95,7 @@ abstract class base implements type_interface } $this->excluded = [ - (int)$this->user->data['user_id'] => $this->user->data[$user_column], + (int) $this->user->data['user_id'] => $this->user->data[$user_column], ]; $sql = "SELECT user_id, {$user_column} diff --git a/phpBB/phpbb/ban/type/email.php b/phpBB/phpbb/ban/type/email.php index 675eca0112..1a0e515aaa 100644 --- a/phpBB/phpbb/ban/type/email.php +++ b/phpBB/phpbb/ban/type/email.php @@ -14,7 +14,6 @@ namespace phpbb\ban\type; use phpbb\ban\exception\no_valid_emails_exception; -use phpbb\ban\exception\no_valid_users_exception; use phpbb\exception\runtime_exception; class email extends base diff --git a/phpBB/phpbb/ban/type/type_interface.php b/phpBB/phpbb/ban/type/type_interface.php index 4dfc79595f..cc4e0d0790 100644 --- a/phpBB/phpbb/ban/type/type_interface.php +++ b/phpBB/phpbb/ban/type/type_interface.php @@ -13,8 +13,6 @@ namespace phpbb\ban\type; -use phpbb\mimetype\null_guesser; - /** * Interface implemented by all ban types */ diff --git a/phpBB/phpbb/db/migration/data/v330/ban_table_p1.php b/phpBB/phpbb/db/migration/data/v330/ban_table_p1.php index 2b2e027a49..f53f56a590 100644 --- a/phpBB/phpbb/db/migration/data/v330/ban_table_p1.php +++ b/phpBB/phpbb/db/migration/data/v330/ban_table_p1.php @@ -148,7 +148,7 @@ class ban_table_p1 extends \phpbb\db\migration\migration $processed_rows++; $bans[] = [ - 'ban_userid' => ($row['ban_mode'] === 'user') ? (int)$row['ban_item'] : 0, + 'ban_userid' => ($row['ban_mode'] === 'user') ? (int) $row['ban_item'] : 0, 'ban_ip' => ($row['ban_mode'] === 'ip') ? $row['ban_item'] : '', 'ban_email' => ($row['ban_mode'] === 'email') ? $row['ban_item'] : '', 'ban_start' => $row['ban_start'], diff --git a/phpBB/phpbb/db/migration/data/v330/ban_table_p2.php b/phpBB/phpbb/db/migration/data/v330/ban_table_p2.php index de617a05f2..6e1ce894d0 100644 --- a/phpBB/phpbb/db/migration/data/v330/ban_table_p2.php +++ b/phpBB/phpbb/db/migration/data/v330/ban_table_p2.php @@ -35,7 +35,7 @@ class ban_table_p2 extends \phpbb\db\migration\migration 'add_tables' => array( $this->table_prefix . 'banlist' => array( 'COLUMNS' => array( - 'ban_id' => array('ULINT', NULL, 'auto_increment'), + 'ban_id' => array('ULINT', null, 'auto_increment'), 'ban_userid' => array('ULINT', 0), 'ban_ip' => array('VCHAR:40', ''), 'ban_email' => array('VCHAR_UNI:100', ''), diff --git a/tests/dbal/boolean_processor_test.php b/tests/dbal/boolean_processor_test.php index 517c1dcaf8..3352b99c82 100644 --- a/tests/dbal/boolean_processor_test.php +++ b/tests/dbal/boolean_processor_test.php @@ -153,9 +153,9 @@ class phpbb_boolean_processor_test extends phpbb_database_test_case 'LEFT_JOIN' => array( array( 'FROM' => array( - 'phpbb_banlist' => 'b', + 'phpbb_bans' => 'b', ), - 'ON' => 'u.user_id = b.ban_userid', + 'ON' => 'b.ban_item = ' . $db->cast_expr_to_string('u.user_id'), ), ), 'WHERE' => array('AND', @@ -172,6 +172,7 @@ class phpbb_boolean_processor_test extends phpbb_database_test_case array( array('ug.group_id', '=', 1), array('b.ban_id', 'IS_NOT', NULL), + array('b.ban_mode', '=', "'user'"), ), ), ), @@ -290,9 +291,9 @@ class phpbb_boolean_processor_test extends phpbb_database_test_case 'LEFT_JOIN' => array( array( 'FROM' => array( - 'phpbb_banlist' => 'b', + 'phpbb_bans' => 'b', ), - 'ON' => 'u.user_id = b.ban_userid', + 'ON' => 'b.ban_item = ' . $db->cast_expr_to_string('u.user_id'), ), ), 'WHERE' => array('AND', diff --git a/tests/dbal/cross_join_test.php b/tests/dbal/cross_join_test.php index 95bcc0be53..83c4b03a60 100644 --- a/tests/dbal/cross_join_test.php +++ b/tests/dbal/cross_join_test.php @@ -36,9 +36,9 @@ class phpbb_dbal_cross_join_test extends phpbb_database_test_case 'LEFT_JOIN' => array( array( 'FROM' => array( - 'phpbb_banlist' => 'b', + 'phpbb_bans' => 'b', ), - 'ON' => 'u.user_id = b.ban_userid', + 'ON' => 'b.ban_item = ' . $db->cast_expr_to_string('u.user_id'), ), ), 'WHERE' => 'ug.group_id = 1 diff --git a/tests/dbal/fixtures/boolean_processor.xml b/tests/dbal/fixtures/boolean_processor.xml index d31d679f45..9bab747fcf 100644 --- a/tests/dbal/fixtures/boolean_processor.xml +++ b/tests/dbal/fixtures/boolean_processor.xml @@ -1,10 +1,12 @@ - +
ban_id - ban_userid + ban_mode + ban_item 1 + user 2
diff --git a/tests/dbal/fixtures/massmail_crossjoin.xml b/tests/dbal/fixtures/massmail_crossjoin.xml index 1050ba067e..d453d5e9fd 100644 --- a/tests/dbal/fixtures/massmail_crossjoin.xml +++ b/tests/dbal/fixtures/massmail_crossjoin.xml @@ -1,10 +1,12 @@ - +
ban_id - ban_userid + ban_mode + ban_item 1 + user 2
diff --git a/tests/functions/fixtures/banned_users.xml b/tests/functions/fixtures/banned_users.xml index cec3f4e51f..4be21c3201 100644 --- a/tests/functions/fixtures/banned_users.xml +++ b/tests/functions/fixtures/banned_users.xml @@ -1,6 +1,6 @@ - +
ban_useridban_excludeban_end diff --git a/tests/functions_user/delete_user_test.php b/tests/functions_user/delete_user_test.php index 76a0aa50ce..9f4f69d0c2 100644 --- a/tests/functions_user/delete_user_test.php +++ b/tests/functions_user/delete_user_test.php @@ -369,7 +369,7 @@ class phpbb_functions_user_delete_user_test extends phpbb_database_test_case $this->db->sql_freeresult($result); $sql = 'SELECT ban_id - FROM ' . BANLIST_TABLE . ' + FROM ' . BANS_TABLE . ' ORDER BY ban_id ASC'; $result = $this->db->sql_query($sql); $this->assertEquals($expected_ban, $this->db->sql_fetchrowset($result), 'Ban table content is mismatching after deleting a user.'); diff --git a/tests/functions_user/fixtures/delete_user.xml b/tests/functions_user/fixtures/delete_user.xml index 8de2659722..1727b8a783 100644 --- a/tests/functions_user/fixtures/delete_user.xml +++ b/tests/functions_user/fixtures/delete_user.xml @@ -36,31 +36,39 @@
- +
ban_id - ban_userid - ban_email + ban_mode + ban_item + ban_start + ban_endban_reason - ban_give_reason + ban_reason_display 1 + user 2 - - - - - - 2 - 3 - - - - - - 3 + 0 0 + + + 2 + user + 3 + 0 + 0 + + + + + 3 + user + 0 + 0 + 0 +
diff --git a/tests/session/check_ban_test.php b/tests/session/check_ban_test.php index 7b0aac060c..100c039639 100644 --- a/tests/session/check_ban_test.php +++ b/tests/session/check_ban_test.php @@ -33,8 +33,8 @@ class phpbb_session_check_ban_test extends phpbb_session_test_case false, false, false, false, /* should be banned? -> */ false), array('Matching values in the database, should be banned', 4, '127.0.0.1', 'bar@example.org', true, /* should be banned? -> */ true), - array('IP Banned, should be banned', - false, '127.1.1.1', false, false, /* should be banned? -> */ true), + array('IP Banned, should not be banned', + false, '127.1.1.1', false, false, /* should be banned? -> */ false), ); } diff --git a/tests/session/fixtures/sessions_banlist.xml b/tests/session/fixtures/sessions_banlist.xml index e720e35f0a..b6c123e119 100644 --- a/tests/session/fixtures/sessions_banlist.xml +++ b/tests/session/fixtures/sessions_banlist.xml @@ -26,34 +26,46 @@ 1 - +
ban_id - ban_userid - ban_ip - ban_email + ban_mode + ban_itemban_startban_end - ban_excludeban_reason - ban_give_reason + ban_reason_display 2 - 4 + ip 127.0.0.1 - bar@example.org 1111 0 - 0 HAHAHA 1 3 - 0 + ip 127.1.1.1 - 1111 0 + HAHAHA + 1 + + + 4 + user + 4 + 1111 + 0 + HAHAHA + 1 + + + 5 + email + bar@example.org + 1111 0 HAHAHA 1 diff --git a/tests/session/fixtures/sessions_empty.xml b/tests/session/fixtures/sessions_empty.xml index 068951dc4c..4df982ab7f 100644 --- a/tests/session/fixtures/sessions_empty.xml +++ b/tests/session/fixtures/sessions_empty.xml @@ -30,11 +30,13 @@ session_ip session_browser
- +
ban_id - ban_userid - ban_email + ban_mode + ban_item + ban_start + ban_endban_reason - ban_give_reason + ban_reason_display