[ticket/9687] Improve code quality of ban system

PHPBB3-9687
This commit is contained in:
Marc Alexander 2023-07-04 21:23:02 +02:00
parent ec4a53169a
commit 43b35e1560
No known key found for this signature in database
GPG key ID: 50E0D2423696F995
5 changed files with 70 additions and 42 deletions

View file

@ -16,6 +16,7 @@ namespace phpbb\ban;
use phpbb\ban\exception\ban_insert_failed_exception; use phpbb\ban\exception\ban_insert_failed_exception;
use phpbb\ban\exception\invalid_length_exception; use phpbb\ban\exception\invalid_length_exception;
use phpbb\ban\exception\type_not_found_exception; use phpbb\ban\exception\type_not_found_exception;
use phpbb\ban\type\type_interface;
class manager class manager
{ {
@ -73,19 +74,20 @@ class manager
* *
* @return bool * @return bool
*/ */
public function ban($mode, array $items, \DateTimeInterface $start, \DateTimeInterface $end, $reason, $display_reason = '') public function ban(string $mode, array $items, \DateTimeInterface $start, \DateTimeInterface $end, string $reason, string $display_reason = '')
{ {
if ($start > $end && $end->getTimestamp() !== 0) if ($start > $end && $end->getTimestamp() !== 0)
{ {
throw new invalid_length_exception(); // TODO throw new invalid_length_exception(); // TODO
} }
/** @var \phpbb\ban\type\type_interface $ban_mode */ /** @var type_interface $ban_mode */
$ban_mode = $this->find_type($mode); $ban_mode = $this->find_type($mode);
if ($ban_mode === false) if ($ban_mode === false)
{ {
throw new type_not_found_exception(); // TODO throw new type_not_found_exception(); // TODO
} }
if (!empty($this->user)) if (!empty($this->user))
{ {
$ban_mode->set_user($this->user); $ban_mode->set_user($this->user);
@ -151,7 +153,7 @@ class manager
*/ */
public function unban($mode, array $items) public function unban($mode, array $items)
{ {
/** @var \phpbb\ban\type\type_interface $ban_mode */ /** @var type_interface $ban_mode */
$ban_mode = $this->find_type($mode); $ban_mode = $this->find_type($mode);
if ($ban_mode === false) if ($ban_mode === false)
{ {
@ -205,7 +207,7 @@ class manager
foreach ($ban_info as $mode => $ban_rows) foreach ($ban_info as $mode => $ban_rows)
{ {
/** @var \phpbb\ban\type\type_interface $ban_mode */ /** @var type_interface $ban_mode */
$ban_mode = $this->find_type($mode); $ban_mode = $this->find_type($mode);
if ($ban_mode === false) if ($ban_mode === false)
{ {
@ -264,7 +266,7 @@ class manager
*/ */
public function get_bans($mode) public function get_bans($mode)
{ {
/** @var \phpbb\ban\type\type_interface $ban_mode */ /** @var type_interface $ban_mode */
$ban_mode = $this->find_type($mode); $ban_mode = $this->find_type($mode);
if ($ban_mode === false) if ($ban_mode === false)
{ {
@ -298,7 +300,7 @@ class manager
$manual_modes = []; $manual_modes = [];
$where_array = []; $where_array = [];
/** @var \phpbb\ban\type\type_interface $ban_mode */ /** @var type_interface $ban_mode */
foreach ($this->types as $ban_mode) foreach ($this->types as $ban_mode)
{ {
$user_column = $ban_mode->get_user_column(); $user_column = $ban_mode->get_user_column();
@ -345,7 +347,7 @@ class manager
} }
$this->db->sql_freeresult($result); $this->db->sql_freeresult($result);
/** @var \phpbb\ban\type\type_interface $manual_mode */ /** @var type_interface $manual_mode */
foreach ($manual_modes as $manual_mode) foreach ($manual_modes as $manual_mode)
{ {
$mode_banned_users = $manual_mode->get_banned_users(); $mode_banned_users = $manual_mode->get_banned_users();
@ -368,6 +370,39 @@ class manager
}); });
} }
public function get_ban_end(\phpbb\user $user, \DateTimeInterface $ban_start, $length, $end_date): \DateTimeInterface
{
$current_time = $ban_start->getTimestamp();
$end_time = 0;
if ($length)
{
if ($length != -1 || !$end_date)
{
$end_time = max($current_time, $current_time + ($length) * 60);
}
else
{
if (preg_match('/^\d{4}-\d{2}-\d{2}$/', $end_date))
{
$end_time = max(
$current_time,
\DateTime::createFromFormat('Y-m-d', $end_date, $user->timezone)->getTimestamp()
);
}
else
{
trigger_error('LENGTH_BAN_INVALID', E_USER_WARNING);
}
}
}
$ban_end = new \DateTime();
$ban_end->setTimestamp($end_time);
return $ban_end;
}
/** /**
* Sets the current user to exclude from banning * Sets the current user to exclude from banning
* *
@ -388,7 +423,7 @@ class manager
WHERE ban_end > 0 AND ban_end < ' . (int) time(); WHERE ban_end > 0 AND ban_end < ' . (int) time();
$this->db->sql_query($sql); $this->db->sql_query($sql);
/** @var \phpbb\ban\type\type_interface $type */ /** @var type_interface $type */
foreach ($this->types as $type) foreach ($this->types as $type)
{ {
$type->tidy(); $type->tidy();
@ -405,7 +440,7 @@ class manager
*/ */
protected function find_type($mode) protected function find_type($mode)
{ {
/** @var \phpbb\ban\type\type_interface $type */ /** @var type_interface $type */
foreach ($this->types as $type) foreach ($this->types as $type)
{ {
if ($type->get_type() === $mode) if ($type->get_type() === $mode)

View file

@ -13,9 +13,11 @@
namespace phpbb\ban\type; namespace phpbb\ban\type;
use phpbb\db\driver\driver_interface;
abstract class base implements type_interface abstract class base implements type_interface
{ {
/** @var \phpbb\db\driver\driver_interface */ /** @var driver_interface */
protected $db; protected $db;
/** @var array */ /** @var array */
@ -36,12 +38,12 @@ abstract class base implements type_interface
/** /**
* Creates a ban type. * Creates a ban type.
* *
* @param \phpbb\db\driver\driver_interface $db A phpBB DBAL object * @param driver_interface $db A phpBB DBAL object
* @param string $users_table The users table * @param string $users_table The users table
* @param string $sessions_table The sessions table * @param string $sessions_table The sessions table
* @param string $sessions_keys_table The sessions keys table * @param string $sessions_keys_table The sessions keys table
*/ */
public function __construct(\phpbb\db\driver\driver_interface $db, $users_table, $sessions_table, $sessions_keys_table) public function __construct(driver_interface $db, string $users_table, string $sessions_table, string $sessions_keys_table)
{ {
$this->db = $db; $this->db = $db;
$this->users_table = $users_table; $this->users_table = $users_table;
@ -52,7 +54,7 @@ abstract class base implements type_interface
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function set_user(\phpbb\user $user) public function set_user(\phpbb\user $user): void
{ {
// TODO: Implement new logging // TODO: Implement new logging
$this->user = $user; $this->user = $user;
@ -63,6 +65,7 @@ abstract class base implements type_interface
*/ */
public function after_ban(array $data) public function after_ban(array $data)
{ {
$this->logout_affected_users($data['items']);
return $data['items']; return $data['items'];
} }
@ -85,14 +88,14 @@ abstract class base implements type_interface
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function tidy() public function tidy(): void
{ {
} }
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function get_banned_users() public function get_banned_users(): array
{ {
return []; return [];
} }

View file

@ -21,7 +21,7 @@ class email extends base
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function get_type() public function get_type(): string
{ {
return 'email'; return 'email';
} }
@ -29,7 +29,7 @@ class email extends base
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function get_user_column() public function get_user_column(): ?string
{ {
return 'user_email'; return 'user_email';
} }
@ -37,17 +37,7 @@ class email extends base
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function after_ban(array $data) public function prepare_for_storage(array $items): array
{
$this->logout_affected_users($data['items']);
return parent::after_ban($data);
}
/**
* {@inheritDoc}
*/
public function prepare_for_storage(array $items)
{ {
if (!$this->get_excluded()) if (!$this->get_excluded())
{ {

View file

@ -23,7 +23,7 @@ interface type_interface
* *
* @return string * @return string
*/ */
public function get_type(); public function get_type(): string;
/** /**
* Returns the column in the users table which contains * Returns the column in the users table which contains
@ -33,7 +33,7 @@ interface type_interface
* *
* @return string|null * @return string|null
*/ */
public function get_user_column(); public function get_user_column(): ?string;
/** /**
* Sets a user object to the ban type to have it excluded * Sets a user object to the ban type to have it excluded
@ -41,9 +41,9 @@ interface type_interface
* *
* @param \phpbb\user $user An user object * @param \phpbb\user $user An user object
* *
* @return null * @return void
*/ */
public function set_user(\phpbb\user $user); public function set_user(\phpbb\user $user): void;
/** /**
* Gives the possibility to do some clean up after banning. * Gives the possibility to do some clean up after banning.
@ -98,7 +98,7 @@ interface type_interface
* @return array An array of banned users, where the user ids are the keys * @return array An array of banned users, where the user ids are the keys
* and the value is the end of the ban (or 0 if permanent) * and the value is the end of the ban (or 0 if permanent)
*/ */
public function get_banned_users(); public function get_banned_users(): array;
/** /**
* Prepares the given ban items before saving them in the database * Prepares the given ban items before saving them in the database
@ -107,13 +107,13 @@ interface type_interface
* *
* @return array * @return array
*/ */
public function prepare_for_storage(array $items); public function prepare_for_storage(array $items): array;
/** /**
* Does some cleanup work for the banning mode. * Does some cleanup work for the banning mode.
* Is called before banning and unbanning and as cron job. * Is called before banning and unbanning and as cron job.
* *
* @return null * @return void
*/ */
public function tidy(); public function tidy(): void;
} }

View file

@ -24,7 +24,7 @@ class user extends base
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function get_type() public function get_type(): string
{ {
return 'user'; return 'user';
} }
@ -32,7 +32,7 @@ class user extends base
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function get_user_column() public function get_user_column(): string
{ {
return 'user_id'; return 'user_id';
} }
@ -71,7 +71,7 @@ class user extends base
/** /**
* {@inheritDoc} * {@inheritDoc}
*/ */
public function prepare_for_storage(array $items) public function prepare_for_storage(array $items): array
{ {
if (!$this->get_excluded()) if (!$this->get_excluded())
{ {