[ticket/17010] Switch to using table for tracking push subscriptions

PHPBB3-17010

phpBB/config/default/container/services_notification.yml

@@ -255,3 +255,4 @@ services:
             - '%core.root_path%'
             - '%core.php_ext%'
             - '%tables.notification_push%'
+            - '%tables.push_subscriptions%'

phpBB/config/default/container/tables.yml

@@ -51,6 +51,7 @@ parameters:
     tables.profile_fields_data: '%core.table_prefix%profile_fields_data'
     tables.profile_fields_options_language: '%core.table_prefix%profile_fields_lang'
     tables.profile_fields_language: '%core.table_prefix%profile_lang'
+    tables.push_subscriptions: '%core.table_prefix%push_subscriptions'
     tables.ranks: '%core.table_prefix%ranks'
     tables.reports: '%core.table_prefix%reports'
     tables.reports_reasons: '%core.table_prefix%reports_reasons'

phpBB/phpbb/db/migration/data/v400/add_webpush.php

@@ -43,13 +43,18 @@ class add_webpush extends migration
 					],
 					'PRIMARY_KEY' => ['notification_type_id', 'item_id', 'item_parent_id', 'user_id'],
 				],
-			],
+				$this->table_prefix . 'push_subscriptions' => [
-			'add_columns'	=> [
-				$this->table_prefix . 'users' => [
 					'COLUMNS'	=> [
-						'user_push_subscriptions'	=> ['MTEXT_UNI', '']
+						'subscription_id'	=> ['ULINT', null, 'auto_increment'],
+						'user_id'			=> ['ULINT', 0],
+						'device_name'		=> ['VCHAR:64', ''],
+						'endpoint'			=> ['TEXT', ''],
+						'p256dh'			=> ['VCHAR', ''],
+						'auth'				=> ['VCHAR', ''],
+						'encoding'			=> ['VCHAR:32', ''],
 					],
-				],
+					'PRIMARY_KEY' => ['subscription_id', 'user_id'],
+				]
 			],
 		];
 	}
@@ -57,7 +62,10 @@ class add_webpush extends migration
 	public function revert_schema(): array
 	{
 		return [
-			'drop_tables' => [$this->table_prefix . 'notification_push'],
+			'drop_tables' => [
+				$this->table_prefix . 'notification_push',
+				$this->table_prefix . 'push_subscriptions',
+			],
 		];
 	}
 

phpBB/phpbb/notification/method/webpush.php

@@ -39,6 +39,9 @@ class webpush extends \phpbb\notification\method\messenger_base
 	/** @var string Notification web push table */
 	protected $notification_webpush_table;
 
+	/** @var string Notification push subscriptions table */
+	protected $push_subscriptions_table;
+
 	/**
 	 * Notification Method web push constructor
 	 *
@@ -49,8 +52,10 @@ class webpush extends \phpbb\notification\method\messenger_base
 	 * @param string $phpbb_root_path
 	 * @param string $php_ext
 	 * @param string $notification_webpush_table
+	 * @param string $push_subscriptions_table
 	 */
-	public function __construct(user_loader $user_loader, user $user, config $config, driver_interface $db, string $phpbb_root_path, string $php_ext, string $notification_webpush_table)
+	public function __construct(user_loader $user_loader, user $user, config $config, driver_interface $db,string $phpbb_root_path,
+								string $php_ext, string $notification_webpush_table, string $push_subscriptions_table)
 	{
 		parent::__construct($user_loader, $phpbb_root_path, $php_ext);
 
@@ -58,6 +63,7 @@ class webpush extends \phpbb\notification\method\messenger_base
 		$this->config = $config;
 		$this->db = $db;
 		$this->notification_webpush_table = $notification_webpush_table;
+		$this->push_subscriptions_table = $push_subscriptions_table;
 	}
 
 	/**
@@ -148,9 +154,36 @@ class webpush extends \phpbb\notification\method\messenger_base
 		$banned_users = phpbb_get_banned_user_ids($user_ids);
 
 		// Load all the users we need
-		$this->user_loader->load_users(array_diff($user_ids, $banned_users), array(USER_IGNORE));
+		$notify_users = array_diff($user_ids, $banned_users);
+		$this->user_loader->load_users($notify_users, array(USER_IGNORE));
 
-		$web_push = new \Minishlink\WebPush\WebPush();
+		// Get subscriptions for users
+		$user_subscription_map = [];
+		$sql = 'SELECT * FROM ' . $this->push_subscriptions_table . '
+			WHERE ' . $this->db->sql_in_set('user_id', $notify_users) . '
+			GROUP BY user_id';
+		$result = $this->db->sql_query($sql);
+		while ($row = $this->db->sql_fetchrow($result))
+		{
+			if (isset($user_subscriptions[$row['user_id']]))
+			{
+				$user_subscription_map[$row['user_id']] += $row;
+			}
+			else
+			{
+				$user_subscription_map[$row['user_id']] = [$row];
+			}
+		}
+
+		$auth = [
+			'VAPID' => [
+				'subject' => generate_board_url(false),
+				'publicKey' => $this->config['webpush_vapid_public'],
+				'privateKey' => $this->config['webpush_vapid_private'],
+			],
+		];
+
+		$web_push = new \Minishlink\WebPush\WebPush($auth);
 
 		// Time to go through the queue and send emails
 		/** @var type_interface $notification */
@@ -158,7 +191,7 @@ class webpush extends \phpbb\notification\method\messenger_base
 		{
 			$user = $this->user_loader->get_user($notification->user_id);
 
-			$user_subscriptions = sanitizer::decode($this->user->data['user_push_subscriptions']);
+			$user_subscriptions = $user_subscription_map[$notification->user_id] ?? [];
 
 			if ($user['user_type'] == USER_INACTIVE && $user['user_inactive_reason'] == INACTIVE_MANUAL
 				|| empty($user_subscriptions))
@@ -181,8 +214,17 @@ class webpush extends \phpbb\notification\method\messenger_base
 			{
 				try
 				{
-					$push_subscription = \Minishlink\WebPush\Subscription::create($subscription);
+					$push_subscription = \Minishlink\WebPush\Subscription::create([
-					$web_push->queueNotification($push_subscription, $json_data);
+						'endpoint'			=> $subscription['endpoint'],
+						'keys'				=> [
+							'p256dh'	=> $subscription['p256dh'],
+							'auth'		=> $subscription['auth'],
+						],
+						'contentEncoding'	=> !empty($subscription['encoding']) ? $subscription['encoding'] : null,
+					]);
+					//$web_push->queueNotification($push_subscription, $json_data);
+					$foo = $web_push->sendOneNotification($push_subscription, $json_data);
+					$meh = 2;
 				}
 				catch (\ErrorException $exception)
 				{
@@ -193,14 +235,6 @@ class webpush extends \phpbb\notification\method\messenger_base
 		}
 
 		// @todo: Try offloading to after request
-		try
-		{
-			$web_push->flush();
-		}
-		catch (\ErrorException $exception)
-		{
-			// @todo: Add to error log if we can't flush ...
-		}
 
 		// We're done, empty the queue
 		$this->empty_queue();