Skip to content

Commit

Permalink
[ticket/17010] Increase test coverage
Browse files Browse the repository at this point in the history 
PHPBB3-17010
  • Loading branch information
@marc1706
marc1706 committed Feb 27, 2024
1 parent f36542c commit fc14c5f
Show file tree
Hide file tree
Showing 3 changed files with 204 additions and 18 deletions.
69 changes: 61 additions & 8 deletions phpBB/phpbb/notification/method/webpush.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,21 +239,27 @@ protected function notify_using_webpush(): void
}

// Remove any subscriptions that couldn't be queued, i.e. that have invalid data
if (count($remove_subscriptions))
{
$sql = 'DELETE FROM ' . $this->push_subscriptions_table . '
WHERE ' . $this->db->sql_in_set('subscription_id', $remove_subscriptions);
$this->db->sql_query($sql);
}
$this->remove_subscriptions($remove_subscriptions);

// List to fill with expired subscriptions based on return
$expired_endpoints = [];

try
{
foreach ($web_push->flush($number_of_notifications) as $report)
{
if (!$report->isSuccess())
{
$report_data = \phpbb\json\sanitizer::sanitize($report->jsonSerialize());
$this->log->add('admin', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$report_data['reason']]);
// Fill array of endpoints to remove if subscription has expired
if ($report->isSubscriptionExpired())
{
$expired_endpoints = $report->getEndpoint();
}
else
{
$report_data = \phpbb\json\sanitizer::sanitize($report->jsonSerialize());
$this->log->add('admin', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$report_data['reason']]);
}
}
}
}
Expand All @@ -262,6 +268,8 @@ protected function notify_using_webpush(): void
$this->log->add('critical', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$exception->getMessage()]);
}

$this->clean_expired_subscriptions($user_subscription_map, $expired_endpoints);

// We're done, empty the queue
$this->empty_queue();
}
Expand Down Expand Up @@ -373,4 +381,49 @@ protected function get_user_subscription_map(array $notify_users): array

return $user_subscription_map;
}

/**
* Remove subscriptions
*
* @param array $subscription_ids Subscription ids to remove
* @return void
*/
public function remove_subscriptions(array $subscription_ids): void
{
if (count($subscription_ids))
{
$sql = 'DELETE FROM ' . $this->push_subscriptions_table . '
WHERE ' . $this->db->sql_in_set('subscription_id', $subscription_ids);
$this->db->sql_query($sql);
}
}

/**
* Clean expired subscriptions from the database
*
* @param array $user_subscription_map User subscription map
* @param array $expired_endpoints Expired endpoints
* @return void
*/
protected function clean_expired_subscriptions(array $user_subscription_map, array $expired_endpoints): void
{
if (!count($expired_endpoints))
{
return;
}

$remove_subscriptions = [];
foreach ($expired_endpoints as $endpoint)
{
foreach ($user_subscription_map as $user_id => $subscriptions)
{
if (isset($subscriptions['endpoint']) && $subscriptions['endpoint'] == $endpoint)
{
$remove_subscriptions[] = $subscriptions[$endpoint]['subscription_id'];
}
}
}

$this->remove_subscriptions($remove_subscriptions);
}
}
6 changes: 6 additions & 0 deletions tests/notification/fixtures/webpush_notification.type.post.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@
<column>username_clean</column>
<column>user_permissions</column>
<column>user_sig</column>
<row>
<value>1</value>
<value>Anonymous</value>
<value></value>
<value></value>
</row>
<row>
<value>2</value>
<value>poster</value>
Expand Down
147 changes: 137 additions & 10 deletions tests/notification/notification_method_webpush_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ class notification_method_webpush_test extends phpbb_tests_notification_base
/** @var webpush */
protected $notification_method_webpush;

/** @var \phpbb\language\language */
protected $language;

/** @var \phpbb\log\log_interface */
protected $log;

public function getDataSet()
{
return $this->createXMLDataSet(__DIR__ . '/fixtures/webpush_notification.type.post.xml');
Expand Down Expand Up @@ -97,9 +103,11 @@ protected function setUp(): void
'webpush_vapid_private' => self::VAPID_KEYS['privateKey'],
]);
$lang_loader = new \phpbb\language\language_file_loader($phpbb_root_path, $phpEx);
$language = new \phpbb\language\language($lang_loader);
$user = new \phpbb\user($language, '\phpbb\datetime');
$this->language = new \phpbb\language\language($lang_loader);
$this->language->add_lang('acp/common');
$user = new \phpbb\user($this->language, '\phpbb\datetime');
$this->user = $user;
$this->user->data['user_options'] = 230271;
$this->user_loader = new \phpbb\user_loader($avatar_helper, $this->db, $phpbb_root_path, $phpEx, 'phpbb_users');
$auth = $this->auth = new phpbb_mock_notifications_auth();
$this->phpbb_dispatcher = new phpbb_mock_event_dispatcher();
Expand All @@ -113,20 +121,22 @@ protected function setUp(): void
$phpbb_root_path,
$phpEx
);
$phpbb_log = new \phpbb\log\dummy();

$log_table = 'phpbb_log';
$this->log = new \phpbb\log\log($this->db, $user, $auth, $this->phpbb_dispatcher, $phpbb_root_path, 'adm/', $phpEx, $log_table);

$phpbb_container = $this->container = new ContainerBuilder();
$loader = new YamlFileLoader($phpbb_container, new FileLocator(__DIR__ . '/fixtures'));
$loader->load('services_notification.yml');
$phpbb_container->set('user_loader', $this->user_loader);
$phpbb_container->set('user', $user);
$phpbb_container->set('language', $language);
$phpbb_container->set('language', $this->language);
$phpbb_container->set('config', $this->config);
$phpbb_container->set('dbal.conn', $this->db);
$phpbb_container->set('auth', $auth);
$phpbb_container->set('cache.driver', $cache_driver);
$phpbb_container->set('cache', $cache);
$phpbb_container->set('log', $phpbb_log);
$phpbb_container->set('log', $this->log);
$phpbb_container->set('text_formatter.utils', new \phpbb\textformatter\s9e\utils());
$phpbb_container->set('dispatcher', $this->phpbb_dispatcher);
$phpbb_container->setParameter('core.root_path', $phpbb_root_path);
Expand Down Expand Up @@ -158,7 +168,7 @@ protected function setUp(): void
$collection->add('ban.type.email');
$collection->add('ban.type.user');
$collection->add('ban.type.ip');
$ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $language, $phpbb_log, $user, 'phpbb_bans', 'phpbb_users');
$ban_manager = new \phpbb\ban\manager($collection, new \phpbb\cache\driver\dummy(), $this->db, $this->language, $this->log, $user, 'phpbb_bans', 'phpbb_users');
$phpbb_container->set('ban.manager', $ban_manager);

$this->notification_method_webpush = new \phpbb\notification\method\webpush(
Expand All @@ -183,7 +193,7 @@ protected function setUp(): void
$this->phpbb_dispatcher,
$this->db,
$this->cache,
$language,
$this->language,
$this->user,
'phpbb_notification_types',
'phpbb_user_notifications'
Expand Down Expand Up @@ -400,7 +410,122 @@ public function test_get_subscription($notification_type, $post_data, $expected_
}
}

protected function create_subscription_for_user($user_id): array
/**
* @dataProvider data_notification_webpush
*/
public function test_notify_empty_queue($notification_type, $post_data, $expected_users): void
{
foreach ($expected_users as $user_id => $user_data)
{
$this->create_subscription_for_user($user_id);
}

$post_data = array_merge([
'post_time' => 1349413322,
'poster_id' => 1,
'topic_title' => '',
'post_subject' => '',
'post_username' => '',
'forum_name' => '',
],

$post_data);
$notification_options = [
'item_id' => $post_data['post_id'],
'item_parent_id' => $post_data['topic_id'],
];

$notified_users = $this->notification_method_webpush->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options);
$this->assertEquals(0, count($notified_users), 'Assert no user has been notified yet');

$this->notifications->add_notifications($notification_type, $post_data);

$notified_users = $this->notification_method_webpush->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options);
$this->assertEquals($expected_users, $notified_users, 'Assert that expected users have been notified');

$post_data['post_id']++;
$notification_options['item_id'] = $post_data['post_id'];
$post_data['post_time'] = 1349413323;

$this->notifications->add_notifications($notification_type, $post_data);

$notified_users2 = $this->notification_method_webpush->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options);
$this->assertEquals($expected_users, $notified_users2, 'Assert that expected users stay the same after replying to same topic');
}

/**
* @dataProvider data_notification_webpush
*/
public function test_notify_invalid_endpoint($notification_type, $post_data, $expected_users): void
{
$subscription_info = [];
foreach ($expected_users as $user_id => $user_data)
{
$subscription_info[$user_id][] = $this->create_subscription_for_user($user_id);
}

// Create second subscription for first user ID passed
if (count($expected_users))
{
$first_user_id = array_key_first($expected_users);
$first_user_sub = $this->create_subscription_for_user($first_user_id, true);
$subscription_info[$first_user_id][] = $first_user_sub;
}

$post_data = array_merge([
'post_time' => 1349413322,
'poster_id' => 1,
'topic_title' => '',
'post_subject' => '',
'post_username' => '',
'forum_name' => '',
],

$post_data);
$notification_options = [
'item_id' => $post_data['post_id'],
'item_parent_id' => $post_data['topic_id'],
];

$notified_users = $this->notification_method_webpush->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options);
$this->assertEquals(0, count($notified_users), 'Assert no user has been notified yet');

foreach ($expected_users as $user_id => $data)
{
$messages = $this->get_messages_for_subscription($subscription_info[$user_id][0]['clientHash']);
$this->assertEmpty($messages);
}

$this->notifications->add_notifications($notification_type, $post_data);

$notified_users = $this->notification_method_webpush->get_notified_users($this->notifications->get_notification_type_id($notification_type), $notification_options);
$this->assertEquals($expected_users, $notified_users, 'Assert that expected users have been notified');

foreach ($expected_users as $user_id => $data)
{
$messages = $this->get_messages_for_subscription($subscription_info[$user_id][0]['clientHash']);
$this->assertNotEmpty($messages);
}

if (isset($first_user_sub))
{
$admin_logs = $this->log->get_logs('admin');
$this->db->sql_query('DELETE FROM phpbb_log'); // Clear logs
$this->assertCount(1, $admin_logs, 'Assert that an admin log was created for invalid endpoint');

$log_entry = $admin_logs[0];

$this->assertStringStartsWith('<strong>Web Push message could not be sent:</strong>', $log_entry['action']);
$this->assertStringContainsString('400', $log_entry['action']);
}
}

public function test_get_type(): void
{
$this->assertEquals('notification.method.webpush', $this->notification_method_webpush->get_type());
}

protected function create_subscription_for_user($user_id, bool $invalidate_endpoint = false): array
{
$client = new \GuzzleHttp\Client();
try
Expand All @@ -420,8 +545,10 @@ protected function create_subscription_for_user($user_id): array
$this->assertStringStartsWith('http://localhost:9012/notify/', $subscription_data['endpoint']);
$this->assertIsArray($subscription_data['keys']);

// Add subscription data to admin user (user id 2)

if ($invalidate_endpoint)
{
$subscription_data['endpoint'] .= 'invalid';
}

$push_subscriptions_table = $this->container->getParameter('tables.push_subscriptions');

Expand Down

0 comments on commit fc14c5f

Please sign in to comment.