Merge pull request #25 from iMattPro/updates

Add updates for handling expired subs

notification/method/webpush.php

@@ -238,21 +238,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 = 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 = json_sanitizer::sanitize($report->jsonSerialize());
+						$this->log->add('admin', ANONYMOUS, '', 'LOG_WEBPUSH_MESSAGE_FAIL', false, [$report_data['reason']]);
+					}
 				}
 			}
 		}
@@ -261,6 +267,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();
 	}
@@ -372,4 +380,52 @@ 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 $subscriptions)
+			{
+				foreach ($subscriptions as $subscription)
+				{
+					if (isset($subscription['endpoint']) && $subscription['endpoint'] == $endpoint)
+					{
+						$remove_subscriptions[] = $subscription['subscription_id'];
+					}
+				}
+			}
+		}
+
+		$this->remove_subscriptions($remove_subscriptions);
+	}
 }

package.json

@@ -3,6 +3,6 @@
 	"author": "phpBB Limited",
 	"license": "GPL-2.0",
 	"devDependencies": {
-		"web-push-testing": "^1.0.0"
+		"web-push-testing": "^1.1.1"
 	}
 }

tests/notification/fixtures/webpush_notification.type.post.xml

@@ -2,6 +2,8 @@
 <dataset>
 	<table name="phpbb_banlist">
 	</table>
+	<table name="phpbb_log">
+	</table>
 	<table name="phpbb_forums_watch">
 		<column>forum_id</column>
 		<column>user_id</column>
@@ -135,6 +137,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>