fix(federation): Fix the handling of retrying OCM notifications when the remote was not available

Signed-off-by: Joas Schilling <coding@schilljs.com>

6 files changed, 370 insertions, 163 deletions

diff --git a/lib/BackgroundJob/RetryJob.php b/lib/BackgroundJob/RetryJob.php
deleted file mode 100644
index 591f88611..000000000
--- a/lib/BackgroundJob/RetryJob.php
+++ /dev/null
@@ -1,100 +0,0 @@
-<?php
-
-declare(strict_types=1);
-/**
- * @copyright Copyright (c) 2016, ownCloud, Inc.
- * @copyright Copyright (c) 2021 Gary Kim <gary@garykim.dev>
- *
- * @author Bjoern Schiessle <bjoern@schiessle.org>
- * @author Björn Schießle <bjoern@schiessle.org>
- * @author Joas Schilling <coding@schilljs.com>
- * @author Lukas Reschke <lukas@statuscode.ch>
- * @author Morris Jobke <hey@morrisjobke.de>
- * @author Roeland Jago Douma <roeland@famdouma.nl>
- * @author Gary Kim <gary@garykim.dev>
- *
- * @license AGPL-3.0
- *
- * This code is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Affero General Public License, version 3,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU Affero General Public License for more details.
- *
- * You should have received a copy of the GNU Affero General Public License, version 3,
- * along with this program. If not, see <http://www.gnu.org/licenses/>
- *
- */
-namespace OCA\Talk\BackgroundJob;
-
-use OCA\Talk\Federation\BackendNotifier;
-use OCP\AppFramework\Utility\ITimeFactory;
-use OCP\BackgroundJob\IJobList;
-use OCP\BackgroundJob\Job;
-use OCP\ILogger;
-
-/**
- * Class RetryJob
- *
- * Background job to re-send update of federated re-shares to the remote server in
- * case the server was not available on the first try
- *
- * @package OCA\Talk\BackgroundJob
- */
-class RetryJob extends Job {
-
-	/** @var int max number of attempts to send the request */
-	private int $maxTry = 20;
-
-
-	public function __construct(
-		private BackendNotifier $backendNotifier,
-		ITimeFactory $timeFactory,
-	) {
-		parent::__construct($timeFactory);
-	}
-
-	/**
-	 * run the job, then remove it from the jobList
-	 *
-	 * @param IJobList $jobList
-	 * @param ILogger|null $logger
-	 */
-	public function execute(IJobList $jobList, ?ILogger $logger = null): void {
-		if (((int)$this->argument['try']) > $this->maxTry) {
-			$jobList->remove($this, $this->argument);
-			return;
-		}
-		if ($this->shouldRun($this->argument)) {
-			parent::execute($jobList, $logger);
-			$jobList->remove($this, $this->argument);
-		}
-	}
-
-	protected function run($argument): void {
-		$remote = $argument['remote'];
-		$data = json_decode($argument['data'], true, flags: JSON_THROW_ON_ERROR);
-		$try = (int)$argument['try'] + 1;
-
-		$this->backendNotifier->sendUpdateDataToRemote($remote, $data, $try);
-	}
-
-	/**
-	 * test if it is time for the next run
-	 *
-	 * @param array $argument
-	 * @return bool
-	 */
-	protected function shouldRun(array $argument): bool {
-		$lastRun = (int)$argument['lastRun'];
-		$try = (int)$argument['try'];
-		return (($this->time->getTime() - $lastRun) > $this->nextRunBreak($try));
-	}
-
-	protected function nextRunBreak(int $try): int {
-		return min(($try + 1) * 300, 3600);
-	}
-}
diff --git a/lib/BackgroundJob/RetryNotificationsJob.php b/lib/BackgroundJob/RetryNotificationsJob.php
new file mode 100644
index 000000000..de0454a9a
--- /dev/null
+++ b/lib/BackgroundJob/RetryNotificationsJob.php
@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCA\Talk\BackgroundJob;
+
+use OCA\Talk\Federation\BackendNotifier;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\TimedJob;
+
+/**
+ * Retry to send OCM notifications
+ */
+class RetryNotificationsJob extends TimedJob {
+	public function __construct(
+		private BackendNotifier $backendNotifier,
+		ITimeFactory $timeFactory,
+	) {
+		parent::__construct($timeFactory);
+
+		// Every time the jobs run
+		$this->setInterval(1);
+	}
+
+	protected function run($argument): void {
+		$this->backendNotifier->retrySendingFailedNotifications($this->time->getDateTime());
+	}
+}
diff --git a/lib/Federation/BackendNotifier.php b/lib/Federation/BackendNotifier.php
index ccb422b23..78401e319 100644
--- a/lib/Federation/BackendNotifier.php
+++ b/lib/Federation/BackendNotifier.php
@@ -27,15 +27,16 @@ namespace OCA\Talk\Federation;
 
 use OCA\FederatedFileSharing\AddressHandler;
 use OCA\Federation\TrustedServers;
-use OCA\Talk\BackgroundJob\RetryJob;
 use OCA\Talk\Config;
 use OCA\Talk\Exceptions\RoomHasNoModeratorException;
 use OCA\Talk\Model\Attendee;
+use OCA\Talk\Model\RetryNotification;
+use OCA\Talk\Model\RetryNotificationMapper;
 use OCA\Talk\Room;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Services\IAppConfig;
-use OCP\BackgroundJob\IJobList;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\DB\Exception;
 use OCP\Federation\ICloudFederationFactory;
 use OCP\Federation\ICloudFederationNotification;
@@ -56,12 +57,13 @@ class BackendNotifier {
 		private AddressHandler $addressHandler,
 		private LoggerInterface $logger,
 		private ICloudFederationProviderManager $federationProviderManager,
-		private IJobList $jobList,
 		private IUserManager $userManager,
 		private IURLGenerator $url,
 		private IAppManager $appManager,
 		private Config $talkConfig,
 		private IAppConfig $appConfig,
+		private RetryNotificationMapper $retryNotificationMapper,
+		private ITimeFactory $timeFactory,
 	) {
 	}
 
@@ -192,22 +194,7 @@ class BackendNotifier {
 			]
 		);
 
-		try {
-			$response = $this->federationProviderManager->sendCloudNotification($remote, $notification);
-			if ($response->getStatusCode() === Http::STATUS_CREATED) {
-				return true;
-			}
-
-			$this->logger->warning("Failed to send share accepted notification for share from $remote, received status code {code}\n{body}", [
-				'code' => $response->getStatusCode(),
-				'body' => (string) $response->getBody(),
-			]);
-
-			return false;
-		} catch (OCMProviderException $e) {
-			$this->logger->error("Failed to send share accepted notification for share from $remote, received OCMProviderException", ['exception' => $e]);
-			return false;
-		}
+		return $this->sendUpdateToRemote($remote, $notification, retry: false);
 	}
 
 	/**
@@ -219,7 +206,7 @@ class BackendNotifier {
 		int $remoteAttendeeId,
 		#[SensitiveParameter]
 		string $accessToken,
-	): bool {
+	): void {
 		$remote = $this->prepareRemoteUrl($remoteServerUrl);
 
 		$notification = $this->cloudFederationFactory->getCloudFederationNotification();
@@ -234,22 +221,7 @@ class BackendNotifier {
 			]
 		);
 
-		try {
-			$response = $this->federationProviderManager->sendCloudNotification($remote, $notification);
-			if ($response->getStatusCode() === Http::STATUS_CREATED) {
-				return true;
-			}
-
-			$this->logger->warning("Failed to send share declined notification for share from $remote, received status code {code}\n{body}", [
-				'code' => $response->getStatusCode(),
-				'body' => (string) $response->getBody(),
-			]);
-
-			return false;
-		} catch (OCMProviderException $e) {
-			$this->logger->error("Failed to send share declined notification for share from $remote, received OCMProviderException", ['exception' => $e]);
-			return false;
-		}
+		$this->sendUpdateToRemote($remote, $notification);
 	}
 
 	public function sendRemoteUnShare(
@@ -344,29 +316,11 @@ class BackendNotifier {
 		$this->sendUpdateToRemote($remote, $notification);
 	}
 
-	/**
-	 * @param string $remote
-	 * @param array{notificationType: string, resourceType: string, providerId: string, notification: array} $data
-	 * @param int $try
-	 * @return void
-	 * @internal Used to send retries in background jobs
-	 */
-	public function sendUpdateDataToRemote(string $remote, array $data, int $try): void {
-		$notification = $this->cloudFederationFactory->getCloudFederationNotification();
-		$notification->setMessage(
-			$data['notificationType'],
-			$data['resourceType'],
-			$data['providerId'],
-			$data['notification']
-		);
-		$this->sendUpdateToRemote($remote, $notification, $try);
-	}
-
-	protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0): void {
+	protected function sendUpdateToRemote(string $remote, ICloudFederationNotification $notification, int $try = 0, bool $retry = true): bool {
 		try {
 			$response = $this->federationProviderManager->sendCloudNotification($remote, $notification);
 			if ($response->getStatusCode() === Http::STATUS_CREATED) {
-				return;
+				return true;
 			}
 
 			$this->logger->warning("Failed to send notification for share from $remote, received status code {code}\n{body}", [
@@ -377,14 +331,84 @@ class BackendNotifier {
 			$this->logger->error("Failed to send notification for share from $remote, received OCMProviderException", ['exception' => $e]);
 		}
 
-		$this->jobList->add(
-			RetryJob::class,
-			[
-				'remote' => $remote,
-				'data' => json_encode($notification->getMessage(), JSON_THROW_ON_ERROR),
-				'try' => $try,
-			]
+		if ($retry && $try === 0) {
+			$now = $this->timeFactory->getTime();
+			$now += $this->getRetryDelay(1);
+
+			// Talk data
+			$retryNotification = new RetryNotification();
+			$retryNotification->setRemoteServer($remote);
+			$retryNotification->setNumAttempts(1);
+			$retryNotification->setNextRetry($this->timeFactory->getDateTime('@' . $now));
+
+			// OCM notification data
+			$data = $notification->getMessage();
+			$retryNotification->setNotificationType($data['notificationType']);
+			$retryNotification->setResourceType($data['resourceType']);
+			$retryNotification->setProviderId($data['providerId']);
+			$retryNotification->setNotification(json_encode($data['notification']));
+
+			$this->retryNotificationMapper->insert($retryNotification);
+		}
+
+		return false;
+	}
+
+	public function retrySendingFailedNotifications(\DateTimeInterface $dueDateTime): void {
+		$retryNotifications = $this->retryNotificationMapper->getAllDue($dueDateTime);
+
+		foreach ($retryNotifications as $retryNotification) {
+			$this->retrySendingFailedNotification($retryNotification);
+		}
+	}
+
+	protected function retrySendingFailedNotification(RetryNotification $retryNotification): void {
+		$notification = $this->cloudFederationFactory->getCloudFederationNotification();
+		$notification->setMessage(
+			$retryNotification->getNotificationType(),
+			$retryNotification->getResourceType(),
+			$retryNotification->getProviderId(),
+			json_decode($retryNotification->getNotification(), true, flags: JSON_THROW_ON_ERROR),
 		);
+
+		$success = $this->sendUpdateToRemote($retryNotification->getRemoteServer(), $notification, $retryNotification->getNumAttempts());
+
+		if ($success) {
+			$this->retryNotificationMapper->delete($retryNotification);
+		} elseif ($retryNotification->getNumAttempts() === RetryNotification::MAX_NUM_ATTEMPTS) {
+			$this->logger->error('Failed to send notification to ' . $retryNotification->getRemoteServer() . ' ' . RetryNotification::MAX_NUM_ATTEMPTS . ' times, giving up!');
+			$this->retryNotificationMapper->delete($retryNotification);
+		} else {
+			$retryNotification->setNumAttempts($retryNotification->getNumAttempts() + 1);
+
+			$now = $this->timeFactory->getTime();
+			$now += $this->getRetryDelay($retryNotification->getNumAttempts());
+
+			$retryNotification->setNextRetry($this->timeFactory->getDateTime('@' . $now));
+			$this->retryNotificationMapper->update($retryNotification);
+		}
+	}
+
+	/**
+	 * First 5 attempts are retried on the next cron run.
+	 * Attempts 6-10 we back off to cover slightly longer maintenance/downtimes (5 minutes * per attempt)
+	 * And the last tries 11-20 are retried with ~8 hours delay
+	 *
+	 * This means the last retry is after ~84 hours so a downtime from Friday to Monday would be covered
+	 */
+	protected function getRetryDelay(int $attempt): int {
+		if ($attempt < 5) {
+			// Retry after "attempt" minutes
+			return 5 * 60;
+		}
+
+		if ($attempt > 10) {
+			// Retry after 8 hours
+			return 8 * 3600;
+		}
+
+		// Retry after "attempt" * 5 minutes
+		return $attempt * 5 * 60;
 	}
 
 	protected function prepareRemoteUrl(string $remote): string {
diff --git a/lib/Migration/Version19000Date20240312105627.php b/lib/Migration/Version19000Date20240312105627.php
new file mode 100644
index 000000000..c8e3b45e5
--- /dev/null
+++ b/lib/Migration/Version19000Date20240312105627.php
@@ -0,0 +1,106 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * Add table to queue federation notifications to retry
+ */
+class Version19000Date20240312105627 extends SimpleMigrationStep {
+	public function __construct(
+		protected IDBConnection $connection,
+	) {
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->createTable('talk_retry_ocm');
+		$table->addColumn('id', Types::BIGINT, [
+			'autoincrement' => true,
+			'notnull' => true,
+		]);
+		$table->addColumn('remote_server', Types::STRING, [
+			'notnull' => true,
+			'length' => 255,
+		]);
+		$table->addColumn('num_attempts', Types::INTEGER, [
+			'default' => 0,
+			'unsigned' => true,
+		]);
+		$table->addColumn('next_retry', Types::DATETIME, [
+			'notnull' => false,
+		]);
+		$table->addColumn('notification_type', Types::STRING, [
+			'notnull' => true,
+			'length' => 64,
+		]);
+		$table->addColumn('resource_type', Types::STRING, [
+			'notnull' => true,
+			'length' => 64,
+		]);
+		$table->addColumn('provider_id', Types::STRING, [
+			'notnull' => true,
+			'length' => 64,
+		]);
+		$table->addColumn('notification', Types::TEXT, [
+			'notnull' => true,
+		]);
+
+
+		$table->setPrimaryKey(['id']);
+		$table->addIndex(['next_retry'], 'talk_retry_ocm_next');
+
+		return $schema;
+	}
+
+	/**
+	 * Remove legacy RetryJobs
+	 */
+	public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
+		/** @psalm-suppress UndefinedClass */
+		$formerClassName = \OCA\Talk\BackgroundJob\RetryJob::class;
+
+		$query = $this->connection->getQueryBuilder();
+		$query->delete('jobs')
+			->where($query->expr()->eq('class', $query->createNamedParameter($formerClassName)));
+		$query->executeStatement();
+	}
+}
diff --git a/lib/Model/RetryNotification.php b/lib/Model/RetryNotification.php
new file mode 100644
index 000000000..d460f934c
--- /dev/null
+++ b/lib/Model/RetryNotification.php
@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Model;
+
+use OCP\AppFramework\Db\Entity;
+
+/**
+ * @method void setRemoteServer(string $remoteServer)
+ * @method string getRemoteServer()
+ * @method void setNumAttempts(int $numAttempts)
+ * @method int getNumAttempts()
+ * @method void setNextRetry(\DateTime $nextRetry)
+ * @method \DateTime getNextRetry()
+ * @method void setNotificationType(string $notificationType)
+ * @method string getNotificationType()
+ * @method void setResourceType(string $resourceType)
+ * @method string getResourceType()
+ * @method void setProviderId(string $providerId)
+ * @method string getProviderId()
+ * @method void setNotification(string $notification)
+ * @method string getNotification()
+ */
+class RetryNotification extends Entity {
+	public const MAX_NUM_ATTEMPTS = 20;
+
+	protected string $remoteServer = '';
+	protected int $numAttempts = 0;
+	protected ?\DateTime $nextRetry = null;
+	protected string $notificationType = '';
+	protected string $resourceType = '';
+	protected string $providerId = '';
+	protected string $notification = '';
+
+	public function __construct() {
+		$this->addType('remoteServer', 'string');
+		$this->addType('numAttempts', 'int');
+		$this->addType('nextRetry', 'datetime');
+		$this->addType('notificationType', 'string');
+		$this->addType('resourceType', 'string');
+		$this->addType('providerId', 'string');
+		$this->addType('notification', 'string');
+	}
+}
diff --git a/lib/Model/RetryNotificationMapper.php b/lib/Model/RetryNotificationMapper.php
new file mode 100644
index 000000000..fe518593a
--- /dev/null
+++ b/lib/Model/RetryNotificationMapper.php
@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2024 Joas Schilling <coding@schilljs.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Talk\Model;
+
+use OCP\AppFramework\Db\QBMapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IDBConnection;
+
+/**
+ * @method RetryNotification mapRowToEntity(array $row)
+ * @method RetryNotification findEntity(IQueryBuilder $query)
+ * @method RetryNotification[] findEntities(IQueryBuilder $query)
+ * @template-extends QBMapper<RetryNotification>
+ */
+class RetryNotificationMapper extends QBMapper {
+	public function __construct(
+		IDBConnection $db,
+	) {
+		parent::__construct($db, 'talk_retry_ocm', RetryNotification::class);
+	}
+
+	/**
+	 * @return RetryNotification[]
+	 */
+	public function getAllDue(\DateTimeInterface $dueDateTime, ?int $limit = 500): array {
+		$query = $this->db->getQueryBuilder();
+		$query->select('*')
+			->from($this->getTableName())
+			->where($query->expr()->lte('next_retry', $query->createNamedParameter($dueDateTime, IQueryBuilder::PARAM_DATE), IQueryBuilder::PARAM_DATE));
+
+		if ($limit !== null) {
+			$query->setMaxResults($limit)
+				->orderBy('next_retry', 'ASC')
+				->addOrderBy('id', 'ASC');
+		}
+
+		return $this->findEntities($query);
+	}
+}