Reference resources by id (#39)

* fixed undefined array key bug

* reference resources by id instead of uri

Author: JonathanTreffler

appinfo/info.xml

@@ -20,7 +20,7 @@ The current WebDAV-Push draft is provided by [@bitfireAT](https://github.com/bit
           
 This Nextcloud extension has been developed by [@verdigado](https://github.com/verdigado).
     ]]></description>
-    <version>0.0.2</version>
+    <version>0.0.3</version>
     <licence>agpl</licence>
     <author mail="[email protected]" homepage="https://github.com/bitfireAT/webdav-push">bitfire web engineering</author>
     <author mail="[email protected]">Jonathan Treffler</author>

lib/Dav/ServiceDetectionPlugin.php

@@ -33,9 +33,6 @@
 use OCP\IUser;
 use OCP\IUserSession;
 
-use OCP\AppFramework\Db\DoesNotExistException;
-use OCA\DAV\CalDAV\Calendar;
-
 use Sabre\DAV\INode;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\Server;
@@ -58,7 +55,11 @@ public function propFind(PropFind $propFind, INode $node) {
 			return;
 		}
 
-		if (!($node instanceof Calendar)) {
+		if(($node instanceof \OCA\DAV\CalDAV\Calendar)) {
+			$resourceType = "calendar";
+		} else if($node instanceof \OCA\DAV\CardDAV\AddressBook) {
+			$resourceType = "addressbook";
+		} else {
 			return;
 		}
 
@@ -96,7 +97,7 @@ function () use ($node) {
 
 			//	return "test-return-push";
 			//},
-			$node->getName()
+			$resourceType . "-" . $node->getResourceId(),
 		);
 	}
 }

lib/Dav/SubscriptionManagementPlugin.php

@@ -176,6 +176,14 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
 
 		$node = $this->server->tree->getNodeForPath($this->server->getRequestUri());
 
+		if(($node instanceof \OCA\DAV\CalDAV\Calendar)) {
+			$resourceType = "calendar";
+		} else if($node instanceof \OCA\DAV\CardDAV\AddressBook) {
+			$resourceType = "addressbook";
+		} else {
+			return;
+		}
+
 		$requestBody = $request->getBodyAsString();
 
 		// If this request handler could not deal with this POST request, it
@@ -225,13 +233,13 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
 					} else {
 						$user = $this->userSession->getUser();
 
-						$existingSubscriptionId = $transport->getSubscriptionIdFromOptions($user->getUID(), $node->getName(), $subscriptionOptions);
+						$existingSubscriptionId = $transport->getSubscriptionIdFromOptions($user->getUID(), $resourceType, $node->getResourceId(), $subscriptionOptions);
 
 						if(!is_int($existingSubscriptionId)) {
 							try {
 								// create new subscription entry in db and register with transport. If the transport register fails roll back db transaction
-								$this->atomic(function () use ($user, $node, $subscriptionType, $subscriptionExpires, $subscriptionOptions, &$subscription, $transport, &$errors, &$responseStatus, &$responseContent, &$unsubscribeLink) {
-									$subscription = $this->subscriptionService->create($user->getUID(), $node->getName(), $subscriptionType, $subscriptionExpires);
+								$this->atomic(function () use ($user, $resourceType, $node, $subscriptionType, $subscriptionExpires, $subscriptionOptions, &$subscription, $transport, &$errors, &$responseStatus, &$responseContent, &$unsubscribeLink) {
+									$subscription = $this->subscriptionService->create($user->getUID(), $resourceType, $node->getResourceId(), $subscriptionType, $subscriptionExpires);
 
 									[
 										'success' => $registerSuccess,
@@ -260,8 +268,8 @@ public function httpPost(RequestInterface $request, ResponseInterface $response)
 							// implicitly checks if subscription found by transport is really owned by correct user
 							$subscription = $this->subscriptionService->find($user->getUID(), $existingSubscriptionId);
 
-							// check if subscription found by transport is really for correct collection
-							if($subscription->getCollectionName() !== $node->getName()) {
+							// check if subscription found by transport is really for correct calendar/addressbook
+							if($subscription->getResourceType() !== $resourceType || $subscription->getResourceId() !== $node->getResourceId()) {
 								$errors[] = "subscription update error";
 							} else {
 								[

lib/Db/Subscription.php

@@ -10,13 +10,15 @@
 
 class Subscription extends Entity implements JsonSerializable, TableSerializable {
 	protected $userId;
-	protected $collectionName;
+	protected $resourceType;
+	protected $resourceId;
 	protected $transport;
 	protected $creationTimestamp;
 	protected $expirationTimestamp;
 	protected $failCounter;
 
 	public function __construct() {
+		$this->addType('resourceId','integer');
 		$this->addType('creationTimestamp','integer');
 		$this->addType('expirationTimestamp','integer');
 		$this->addType('failCounter','integer');
@@ -26,7 +28,8 @@ public function jsonSerialize(): array {
 		return [
 			'id' => $this->id,
 			'userId' => $this->userId,
-			'collectionName' => $this->collectionName,
+			'resourceType' => $this->resourceType,
+			'resourceId' => $this->resourceId,
 			'transport' => $this->transport,
 			'creationTimestamp' => $this->creationTimestamp,
 			'expirationTimestamp' => $this->expirationTimestamp,
@@ -37,8 +40,9 @@ public function jsonSerialize(): array {
 	public function tableSerialize(?array $params = null): array {
 		return [
 			'Id' => $this->id,
-			'User Id' => $this->userId,
-			'DAV Collection Name' => $this->collectionName,
+			'User ID' => $this->userId,
+			'Resource Type' => $this->resourceType,
+			'Resource ID' => $this->resourceId,
 			'Transport' => $this->transport,
 			'Creation Timestamp' => $this->creationTimestamp,
 			'Expiration Timestamp' => $this->expirationTimestamp,

lib/Db/SubscriptionMapper.php

@@ -34,16 +34,18 @@ public function find(string $userId, int $id): Subscription {
 	}
 
 	/**
-	 * @param string $collectionName
+	 * @param string resourceType
+	 * @param int resourceId
 	 * @return Subscription[]
 	 */
-	public function findAll(string $collectionName): array {
+	public function findAll(string $resourceType, int $resourceId): array {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
 
 		$qb->select('*')
 			->from(self::TABLENAME)
-			->where($qb->expr()->eq('collection_name', $qb->createNamedParameter($collectionName)));
+			->where($qb->expr()->eq('resource_type', $qb->createNamedParameter($resourceType)))
+			->andWhere($qb->expr()->eq('resource_id', $qb->createNamedParameter($resourceId, IQueryBuilder::PARAM_INT)));
 
 		return $this->findEntities($qb);
 	}
@@ -76,6 +78,8 @@ public function cleanupAll(int $maxFails = 3): int {
 				$qb->expr()->gt('fail_counter', $qb->createNamedParameter($maxFails, IQueryBuilder::PARAM_INT)),
 				$qb->expr()->lt('expiration_timestamp', $qb->createNamedParameter(time(), IQueryBuilder::PARAM_INT)),
 			));
+
+		// TODO: call transport deleteSubscription
 
 		return $qb->executeStatement();
 	}

lib/Db/WebPushSubscriptionMapper.php

@@ -32,13 +32,13 @@ public function findBySubscriptionId(int $subscriptionId): WebPushSubscription {
 		return $this->findEntity($qb);
 	}
 
-    /**
+	/**
 	 * @param string $pushResource
 	 * @return Entity|WebPushSubscription
 	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
 	 * @throws DoesNotExistException
 	 */
-	public function findByPushResource(string $userId, string $collectionName, string $pushResource): WebPushSubscription {
+	public function findByPushResource(string $userId, string $resourceType, int $resourceId, string $pushResource): WebPushSubscription {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('webpush.*')
@@ -48,7 +48,8 @@ public function findByPushResource(string $userId, string $collectionName, strin
 
 		$qb->where($qb->expr()->eq('webpush.push_resource', $qb->createNamedParameter($pushResource)))
 			->andWhere($qb->expr()->eq('subscription.user_id', $qb->createNamedParameter($userId)))
-			->andWhere($qb->expr()->eq('subscription.collection_name', $qb->createNamedParameter($collectionName)));
+			->andWhere($qb->expr()->eq('subscription.resource_type', $qb->createNamedParameter($resourceType)))
+			->andWhere($qb->expr()->eq('subscription.resource_id', $qb->createNamedParameter($resourceId, IQueryBuilder::PARAM_INT)));
 
 		return $this->findEntity($qb);
 	}

lib/Listener/CalendarListener.php

@@ -47,44 +47,45 @@
 
 class CalendarListener implements IEventListener {
 
-    public function __construct(
+	public function __construct(
 		private LoggerInterface $logger,
 		private SubscriptionService $subscriptionService,
 		private TransportManager $transportManager,
 		private ErrorHandlingHelper $errorHandlingHelper,
 		private $userId,
 	) {}
 
-    public function handle(Event $event): void {
-        if (($event instanceOf CalendarObjectCreatedEvent) || ($event instanceOf CalendarObjectDeletedEvent) ||
-            ($event instanceOf CalendarObjectUpdatedEvent) || ($event instanceOf CalendarObjectMovedToTrashEvent) ||
+	public function handle(Event $event): void {
+		if (($event instanceOf CalendarObjectCreatedEvent) || ($event instanceOf CalendarObjectDeletedEvent) ||
+			($event instanceOf CalendarObjectUpdatedEvent) || ($event instanceOf CalendarObjectMovedToTrashEvent) ||
 			($event instanceOf CalendarObjectRestoredEvent)) {
-			$this->notifyAllForCalendar($event->getCalendarData());
-        }
+			$this->notifyAllSubscriptionsToResource("calendar", $event->getCalendarData()['id'], $event->getCalendarData()['{http://sabredav.org/ns}sync-token']);
+		}
 
 		if($event instanceOf CalendarObjectMovedEvent) {
-			$this->notifyAllForCalendar($event->getSourceCalendarData());
-			$this->notifyAllForCalendar($event->getTargetCalendarData());
+			$this->notifyAllSubscriptionsToResource("calendar", $event->getSourceCalendarData()['id'], $event->getSourceCalendarData()['{http://sabredav.org/ns}sync-token']);
+			$this->notifyAllSubscriptionsToResource("calendar", $event->getTargetCalendarData()['id'], $event->getTargetCalendarData()['{http://sabredav.org/ns}sync-token']);
 		}
-    }
-	private function notifyAllForCalendar(array $calendarData): void {
-		$collectionName = $calendarData['uri'];
-		$syncToken = $calendarData['{http://sabredav.org/ns}sync-token'];
-		
-		$subscriptions = $this->subscriptionService->findAll($collectionName);
+	}
 
-		$this->errorHandlingHelper->convertErrorsToExceptions(function () use ($subscriptions, $collectionName, $syncToken) {
+	private function notifyAllSubscriptionsToResource(string $resourceType, int $resourceId, string $syncToken): void {
+		$subscriptions = $this->subscriptionService->findAll($resourceType, $resourceId);
+
+		$this->errorHandlingHelper->convertErrorsToExceptions(function () use ($subscriptions, $resourceType, $resourceId, $syncToken) {
 			foreach($subscriptions as $subscription) {
+				// TODO: The subscription was able to be registered and has not expired yet, that means the user had access to the resource as of recently,
+				//        but we need to check if that is actually still the case here
+
 				$transport = $this->transportManager->getTransport($subscription->getTransport());
 
 				try {
-					$transport->notify($subscription->getId(), $subscription->getUserId(), $collectionName, $syncToken);
+					$transport->notify($subscription->getId(), $subscription->getUserId(), $resourceType, $resourceId, $syncToken);
 					$this->subscriptionService->update($subscription->getUserId(), $subscription->getId(), failCounter: 0);
 				} catch (\Throwable $e) {
 					$this->logger->error("transport " .  $subscription->getTransport() . " failed to deliver notification to subscription " . $subscription->getId() . ". error message: " . $e->getMessage());
 					$this->subscriptionService->update($subscription->getUserId(), $subscription->getId(), failCounter: $subscription->getFailCounter() + 1);
 				}
 			}
-		});		
+		});
 	}
 }

lib/Migration/Version001Date20240515221000.php

@@ -56,9 +56,15 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 				'notnull' => true,
 				'length' => 200,
 			]);
-			$table->addColumn('collection_name', Types::STRING, [
+			// Either calendar or addressbook
+			$table->addColumn('resource_type', 'string', [
 				'notnull' => true,
-				'length' => 100,
+				'length' => 255,
+			]);
+			$table->addColumn('resource_id', Types::BIGINT, [
+				'notnull' => true,
+				'length' => 11,
+				'unsigned' => true,
 			]);
 			$table->addColumn('transport', Types::STRING, [
 				'notnull' => true,

lib/Migration/Version002Date20250119180000.php

@@ -45,12 +45,12 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 		/** @var ISchemaWrapper $schema */
 		$schema = $schemaClosure();
 
-        $table = $schema->getTable(self::SUBSCRIPTIONS_TABLE);
+		$table = $schema->getTable(self::SUBSCRIPTIONS_TABLE);
 
-        $table->addColumn('fail_counter', Types::INTEGER, [
-            'notnull' => true,
-            'default' => 0,
-        ]);
+		$table->addColumn('fail_counter', Types::INTEGER, [
+			'notnull' => true,
+			'default' => 0,
+		]);
 
 		return $schema;
 	}

lib/Migration/Version003Date20250220223000.php

@@ -0,0 +1,92 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\DavPush\Migration;
+
+/**
+ * @copyright 2025 Jonathan Treffler <[email protected]>
+ *
+ * @author Jonathan Treffler <[email protected]>
+ *
+ * @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/>.
+ *
+ */
+
+use Closure;
+use OCP\DB\Types;
+use OCP\DB\ISchemaWrapper;
+use OCP\IDBConnection;
+use OCP\Migration\SimpleMigrationStep;
+use OCP\Migration\IOutput;
+
+use Psr\Log\LoggerInterface;
+
+class Version003Date20250220223000 extends SimpleMigrationStep {
+	public const SUBSCRIPTIONS_TABLE = "dav_push_subscriptions";
+
+	public function __construct(
+		private IDBConnection $connection,
+		private LoggerInterface $logger,
+	) {}
+
+	public function preSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+		$table = $schema->getTable(self::SUBSCRIPTIONS_TABLE);
+
+		// this migration was made specifically to allow a migration path from alpha version 0.0.2 to 0.0.3 (even though our alpha releases generally do not guarantee a migration path)
+		// not needed for instances that started on a later version and therefore never had the collection_name column
+		if($table->hasColumn("collection_name")) {
+			$this->logger->debug("dav_push was initially installed on this instance before alpha version 0.0.3, all subscriptions will be cleared to provide a migration path to more modern versions");
+			// clear all subscriptions created with the old way of referencing calendars
+			$this->connection
+				->getQueryBuilder()
+				->delete(self::SUBSCRIPTIONS_TABLE)
+				->executeStatement();
+		} else {
+			$this->logger->debug("dav_push was initially installed on this instance after alpha version 0.0.3, migration Version003Date20250220223000 is not needed and wil NOOP");
+		}
+	}
+
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+		$table = $schema->getTable(self::SUBSCRIPTIONS_TABLE);
+
+		if($table->hasColumn("collection_name")) {
+			// Either calendar or addressbook
+			$table->addColumn('resource_type', 'string', [
+				'notnull' => true,
+				'length' => 255,
+			]);
+			$table->addColumn('resource_id', Types::BIGINT, [
+				'notnull' => true,
+				'length' => 11,
+				'unsigned' => true,
+			]);
+			$table->dropColumn('collection_name');
+		}
+
+		return $schema;
+	}
+}