conform to spec by updating existing subscription when registering with equivalent options twice instead of creating new subscription

lib/Dav/SubscriptionManagementPlugin.php

@@ -126,20 +126,63 @@ class SubscriptionManagementPlugin extends ServerPlugin {
 			$transport = $this->transportManager->getTransport($subscriptionType);
 
 			if(!is_null($transport)) {
+				[
+					'success' => $validateSuccess,
+					'errors' => $validateErrors,
+				] = $transport->validateOptions($subscriptionOptions);
+
+				if(!$validateSuccess) {
+					if(isset($validateErrors) && !empty($validateErrors)) {
+						$errors = array_merge($errors, $validateErrors);
+					} else {
+						$errors[] = "options validation error";
+					}
+				} else {
+					$existingSubscriptionId = $transport->getSubscriptionIdFromOptions($subscriptionOptions);
+
+					$user = $this->userSession->getUser();
+
+					if(!is_int($existingSubscriptionId)) {
+						// create new subscription entry in db
+						$subscription = $this->subscriptionService->create($user->getUID(), $node->getName(), $subscriptionType, $subscriptionExpires);
+
 						[
 							'success' => $registerSuccess,
-					'error' => $registerError,
+							'errors' => $registerErrors,
 							'responseStatus' => $responseStatus,
 							'response' => $responseContent,
 							'unsubscribeLink' => $unsubscribeLink,
-					'data' => $data
+						] = $transport->registerSubscription($subscription->getId(), $subscriptionOptions);
-				] = $transport->registerSubscription($subscriptionOptions);
 
 						$responseStatus = $responseStatus ?? Http::STATUS_CREATED;
-				$data = $data ?? False;
 
 						if(!$registerSuccess) {
-					$errors[] = $registerError;
+							if(isset($registerErrors) && !empty($registerErrors)) {
+								$errors = array_merge($errors, $registerErrors);
+							} else {
+								$errors[] = "registration error";
+							}
+						}
+					} else {
+						$subscription = $this->subscriptionService->find($user->getUID(), $existingSubscriptionId);
+
+						[
+							'success' => $updateSuccess,
+							'errors' => $updateErrors,
+						] =	$transport->updateSubscription($subscription->getId(), $subscriptionOptions);
+
+						if(!$updateSuccess) {
+							if(isset($updateErrors) && !empty($updateErrors)) {
+								$errors = array_merge($errors, $updateErrors);
+							} else {
+								$errors[] = "subscription update error";
+							}
+						} else {
+							$subscription = $this->subscriptionService->update($user->getUID(), $subscription->getId(), $subscriptionExpires);
+
+							$responseStatus = Http::STATUS_CREATED;
+						}
+					}
 				}
 			} else {
 				$errors[] = $subscriptionType . " transport does not exist";
@@ -148,10 +191,6 @@ class SubscriptionManagementPlugin extends ServerPlugin {
 			if(sizeof($errors) == 0) {
 				$response->setStatus($responseStatus);
 				
-				// create subscription entry in db
-				$user = $this->userSession->getUser();
-				$subscription = $this->subscriptionService->create($user->getUID(), $node->getName(), $subscriptionType, $subscriptionExpires, $data);
-				
 				// generate default unsubscribe link, unless transport requested a custom url
 				$unsubscribeLink = $unsubscribeLink ?? $this->URLGenerator->getAbsoluteURL("/apps/dav_push/subscriptions/" . $subscription->getId());
 				$response->setHeader("Location", $unsubscribeLink);

lib/Db/WebPushSubscriptionMapper.php

@@ -21,7 +21,7 @@ class WebPushSubscriptionMapper extends QBMapper {
 	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
 	 * @throws DoesNotExistException
 	 */
-	public function findBySubscriptionId(int $subscriptionId): Subscription {
+	public function findBySubscriptionId(int $subscriptionId): WebPushSubscription {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')
@@ -37,7 +37,7 @@ class WebPushSubscriptionMapper extends QBMapper {
 	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
 	 * @throws DoesNotExistException
 	 */
-	public function findByPushResource(string $pushResource): Subscription {
+	public function findByPushResource(string $pushResource): WebPushSubscription {
 		/* @var $qb IQueryBuilder */
 		$qb = $this->db->getQueryBuilder();
 		$qb->select('*')

lib/PushTransports/WebPushTransport.php

@@ -57,11 +57,11 @@ class WebPushTransport extends Transport {
 
 		if(isset($pushResource) && $pushResource !== '') {
 			return [
-				'valid' => True,
+				'success' => True,
 			];
 		} else {
 			return [
-				'valid' => False,
+				'success' => False,
 				'errors' => ["push resource not provided"]
 			];
 		}
@@ -104,5 +104,8 @@ class WebPushTransport extends Transport {
 
 	public function updateSubscription($subsciptionId, $options) {
 		// there are no options which can be edited -> NOOP
+		return [
+			'success' => True,
+		];
 	}
 }

lib/Service/SubscriptionService.php

@@ -55,7 +55,7 @@ class SubscriptionService {
 		return $subscription;
 	}
 
-	public function update(string $userId, int $id, ?int $expirationTimestamp, mixed $data) {
+	public function update(string $userId, int $id, ?int $expirationTimestamp) {
 		try {
 			$subscription = $this->mapper->find($userId, $id);
 			
@@ -63,10 +63,6 @@ class SubscriptionService {
 				$subscription->setExpirationTimestamp($expirationTimestamp);
 			}
 
-			if (!is_null($data)) {
-				$subscription->setData(json_encode($data));
-			}
-
 			return $this->mapper->update($subscription);
 		} catch (Exception $e) {
 			$this->handleException($e);