From fcfed793858d44473cc242d73d0ba044cf0786ad Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 1 Mar 2024 19:40:26 +0100 Subject: [PATCH] [ticket/17010] Properly handle expired subscriptions and extend tests PHPBB3-17010 --- phpBB/phpbb/notification/method/webpush.php | 11 +- .../webpush_notification.type.post.xml | 2 + .../notification_method_webpush_test.php | 165 +++++++++++++++++- 3 files changed, 169 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/notification/method/webpush.php b/phpBB/phpbb/notification/method/webpush.php index c71256a29d..e5b8910a07 100644 --- a/phpBB/phpbb/notification/method/webpush.php +++ b/phpBB/phpbb/notification/method/webpush.php @@ -253,7 +253,7 @@ class webpush extends messenger_base implements extended_method_interface // Fill array of endpoints to remove if subscription has expired if ($report->isSubscriptionExpired()) { - $expired_endpoints = $report->getEndpoint(); + $expired_endpoints[] = $report->getEndpoint(); } else { @@ -415,11 +415,14 @@ class webpush extends messenger_base implements extended_method_interface $remove_subscriptions = []; foreach ($expired_endpoints as $endpoint) { - foreach ($user_subscription_map as $user_id => $subscriptions) + foreach ($user_subscription_map as $subscriptions) { - if (isset($subscriptions['endpoint']) && $subscriptions['endpoint'] == $endpoint) + foreach ($subscriptions as $subscription) { - $remove_subscriptions[] = $subscriptions[$endpoint]['subscription_id']; + if (isset($subscription['endpoint']) && $subscription['endpoint'] == $endpoint) + { + $remove_subscriptions[] = $subscription['subscription_id']; + } } } } diff --git a/tests/notification/fixtures/webpush_notification.type.post.xml b/tests/notification/fixtures/webpush_notification.type.post.xml index 896df4e1c1..d59d5b92c4 100644 --- a/tests/notification/fixtures/webpush_notification.type.post.xml +++ b/tests/notification/fixtures/webpush_notification.type.post.xml @@ -1,5 +1,7 @@ + +
forum_iduser_id diff --git a/tests/notification/notification_method_webpush_test.php b/tests/notification/notification_method_webpush_test.php index 3a0a48064c..ff95c74b09 100644 --- a/tests/notification/notification_method_webpush_test.php +++ b/tests/notification/notification_method_webpush_test.php @@ -406,7 +406,7 @@ class notification_method_webpush_test extends phpbb_tests_notification_base foreach ($expected_users as $user_id => $data) { $messages = $this->get_messages_for_subscription($subscription_info[$user_id][0]['clientHash']); - $this->assertNotEmpty($messages); + $this->assertNotEmpty($messages, 'Failed asserting that user ' . $user_id . ' has received messages.'); } } @@ -438,10 +438,10 @@ class notification_method_webpush_test extends phpbb_tests_notification_base $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); + $this->notification_method_webpush->notify(); // should have no effect $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'); + $this->assertEquals(0, count($notified_users), 'Assert no user has been notified yet'); $post_data['post_id']++; $notification_options['item_id'] = $post_data['post_id']; @@ -504,7 +504,7 @@ class notification_method_webpush_test extends phpbb_tests_notification_base foreach ($expected_users as $user_id => $data) { $messages = $this->get_messages_for_subscription($subscription_info[$user_id][0]['clientHash']); - $this->assertNotEmpty($messages); + $this->assertNotEmpty($messages, 'Failed asserting that user ' . $user_id . ' has received messages.'); } if (isset($first_user_sub)) @@ -520,11 +520,139 @@ class notification_method_webpush_test extends phpbb_tests_notification_base } } + /** + * @dataProvider data_notification_webpush + */ + public function test_notify_expired($notification_type, $post_data, $expected_users) + { + $subscription_info = []; + foreach ($expected_users as $user_id => $user_data) + { + $subscription_info[$user_id][] = $this->create_subscription_for_user($user_id); + } + + $expected_delivered_users = $expected_users; + + // Expire subscriptions for first user + if (count($expected_users)) + { + + $first_user_id = array_key_first($expected_users); + $first_user_subs = $subscription_info[$first_user_id]; + unset($expected_delivered_users[$first_user_id]); + $this->expire_subscription($first_user_subs[0]['clientHash']); + } + + $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_delivered_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_delivered_users as $user_id => $data) + { + $messages = $this->get_messages_for_subscription($subscription_info[$user_id][0]['clientHash']); + $this->assertNotEmpty($messages, 'Failed asserting that user ' . $user_id . ' has received messages.'); + } + } + public function test_get_type(): void { $this->assertEquals('notification.method.webpush', $this->notification_method_webpush->get_type()); } + /** + * @dataProvider data_notification_webpush + */ + public function test_prune_notifications($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); + $subscription_info[$first_user_id][] = $this->create_subscription_for_user($first_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'); + + 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, 'Failed asserting that user ' . $user_id . ' has received messages.'); + } + + // Prune notifications with 0 time, shouldn't change anything + $prune_time = time(); + $this->notification_method_webpush->prune_notifications(0); + $this->assertGreaterThanOrEqual($prune_time, $this->config->offsetGet('read_notification_last_gc'), 'Assert that prune time was set'); + + $cur_notifications = $this->get_notifications(); + $this->assertSameSize($cur_notifications, $expected_users, 'Assert that no notifications have been pruned'); + + // Prune only read not supported, will prune all + $this->notification_method_webpush->prune_notifications($prune_time); + $this->assertGreaterThanOrEqual($prune_time, $this->config->offsetGet('read_notification_last_gc'), 'Assert that prune time was set'); + + $cur_notifications = $this->get_notifications(); + $this->assertCount(0, $cur_notifications, 'Assert that no notifications have been pruned'); + } + protected function create_subscription_for_user($user_id, bool $invalidate_endpoint = false): array { $client = new \GuzzleHttp\Client(); @@ -563,6 +691,22 @@ class notification_method_webpush_test extends phpbb_tests_notification_base return $subscription_data; } + protected function expire_subscription(string $client_hash): void + { + $client = new \GuzzleHttp\Client(); + try + { + $response = $client->request('POST', 'http://localhost:9012/expire-subscription/' . $client_hash); + } + catch (\GuzzleHttp\Exception\GuzzleException $exception) + { + $this->fail('Failed expiring subscription with web-push-testing client: ' . $exception->getMessage()); + } + + $subscription_return = \phpbb\json\sanitizer::decode((string) $response->getBody()); + $this->assertEquals(200, $response->getStatusCode(), 'Expected response status to be 200'); + } + protected function get_messages_for_subscription($client_hash): array { $client = new \GuzzleHttp\Client(); @@ -574,7 +718,7 @@ class notification_method_webpush_test extends phpbb_tests_notification_base } catch (\GuzzleHttp\Exception\GuzzleException $exception) { - $this->fail('Failed getting messages from web-push-testing client'); + $this->fail('Failed getting messages from web-push-testing client: ' . $exception->getMessage()); } $response_data = json_decode($response->getBody()->getContents(), true); @@ -584,4 +728,15 @@ class notification_method_webpush_test extends phpbb_tests_notification_base return $response_data['data']['messages']; } + + protected function get_notifications(): array + { + $webpush_table = $this->container->getParameter('tables.notification_push'); + $sql = 'SELECT * FROM ' . $webpush_table; + $result = $this->db->sql_query($sql); + $sql_ary = $this->db->sql_fetchrowset($result); + $this->db->sql_freeresult($result); + + return $sql_ary; + } }