Merge pull request #10920 from nextcloud/bugfix/noid/handle-DoesNotExistException

fix(call): Handle DoesNotExistException when ringing an attendee

docs/call.md

@@ -75,6 +75,7 @@
         + `400 Bad Request` When the actor is not in the call
         + `403 Forbidden` When the current user is not a moderator
         + `404 Not Found` When the conversation could not be found for the participant
+        + `404 Not Found` When the attendee could not be found in the conversation
         + `412 Precondition Failed` When the lobby is active and the user is not a moderator
 
 ## Send SIP dial-out request

lib/Controller/CallController.php

@@ -45,6 +45,7 @@ use OCA\Talk\Service\ParticipantService;
 use OCA\Talk\Service\RecordingService;
 use OCA\Talk\Service\RoomService;
 use OCA\Talk\Service\SIPDialOutService;
+use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
@@ -181,10 +182,11 @@ class CallController extends AEnvironmentAwareController {
 	 * Ring an attendee
 	 *
 	 * @param int $attendeeId ID of the attendee to ring
-	 * @return DataResponse<Http::STATUS_OK|Http::STATUS_BAD_REQUEST, array<empty>, array{}>
+	 * @return DataResponse<Http::STATUS_OK|Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, array<empty>, array{}>
 	 *
 	 * 200: Attendee rang successfully
 	 * 400: Ringing attendee is not possible
+	 * 404: Attendee could not be found
 	 */
 	#[PublicPage]
 	#[RequireCallEnabled]
@@ -199,8 +201,12 @@ class CallController extends AEnvironmentAwareController {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
 		}
 
-		if (!$this->participantService->sendCallNotificationForAttendee($this->room, $this->participant, $attendeeId)) {
+		try {
+			$this->participantService->sendCallNotificationForAttendee($this->room, $this->participant, $attendeeId);
+		} catch (\InvalidArgumentException) {
 			return new DataResponse([], Http::STATUS_BAD_REQUEST);
+		} catch (DoesNotExistException) {
+			return new DataResponse([], Http::STATUS_NOT_FOUND);
 		}
 
 		return new DataResponse();

lib/Service/ParticipantService.php

@@ -1214,16 +1214,24 @@ class ParticipantService {
 		return true;
 	}
 
-	public function sendCallNotificationForAttendee(Room $room, Participant $currentParticipant, int $targetAttendeeId): bool {
+	/**
+	 * @throws \InvalidArgumentException
+	 * @throws DoesNotExistException
+	 */
+	public function sendCallNotificationForAttendee(Room $room, Participant $currentParticipant, int $targetAttendeeId): void {
 		$attendee = $this->attendeeMapper->getById($targetAttendeeId);
 		if ($attendee->getActorType() !== Attendee::ACTOR_USERS) {
-			return false;
+			throw new \InvalidArgumentException('actor-type');
+		}
+
+		if ($attendee->getRoomId() !== $room->getId()) {
+			throw new DoesNotExistException('Room mismatch');
 		}
 
 		$sessions = $this->sessionMapper->findByAttendeeId($targetAttendeeId);
 		foreach ($sessions as $session) {
 			if ($session->getInCall() !== Participant::FLAG_DISCONNECTED) {
-				return false;
+				return;
 			}
 		}
 
@@ -1235,8 +1243,6 @@ class ParticipantService {
 		));
 		$event = new CallNotificationSendEvent($room, $currentParticipant, $target);
 		$this->dispatcher->dispatchTyped($event);
-
-		return true;
 	}
 
 	/**

openapi.json

@@ -4532,6 +4532,34 @@
                                 }
                             }
                         }
+                    },
+                    "404": {
+                        "description": "Attendee could not be found",
+                        "content": {
+                            "application/json": {
+                                "schema": {
+                                    "type": "object",
+                                    "required": [
+                                        "ocs"
+                                    ],
+                                    "properties": {
+                                        "ocs": {
+                                            "type": "object",
+                                            "required": [
+                                                "meta",
+                                                "data"
+                                            ],
+                                            "properties": {
+                                                "meta": {
+                                                    "$ref": "#/components/schemas/OCSMeta"
+                                                },
+                                                "data": {}
+                                            }
+                                        }
+                                    }
+                                }
+                            }
+                        }
                     }
                 }
             }

tests/integration/features/bootstrap/FeatureContext.php

@@ -1784,7 +1784,7 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	}
 
 	/**
-	 * @Then /^user "([^"]*)" pings (user|guest) "([^"]*)" to join call "([^"]*)" with (\d+) \((v4)\)$/
+	 * @Then /^user "([^"]*)" pings (user|guest) "([^"]*)"( attendeeIdPlusOne)? to join call "([^"]*)" with (\d+) \((v4)\)$/
 	 *
 	 * @param string $user
 	 * @param string $actorType
@@ -1793,9 +1793,15 @@ class FeatureContext implements Context, SnippetAcceptingContext {
 	 * @param int $statusCode
 	 * @param string $apiVersion
 	 */
-	public function userPingsAttendeeInRoomTo(string $user, string $actorType, string $actorId, string $identifier, int $statusCode, string $apiVersion): void {
+	public function userPingsAttendeeInRoomTo(string $user, string $actorType, string $actorId, ?string $offset, string $identifier, int $statusCode, string $apiVersion): void {
 		$this->setCurrentUser($user);
-		$this->sendRequest('POST', '/apps/spreed/api/' . $apiVersion . '/call/' . self::$identifierToToken[$identifier] . '/ring/' . self::$userToAttendeeId[$identifier][$actorType . 's'][$actorId]);
+
+		$attendeeId = self::$userToAttendeeId[$identifier][$actorType . 's'][$actorId];
+		if ($offset) {
+			$attendeeId++;
+		}
+
+		$this->sendRequest('POST', '/apps/spreed/api/' . $apiVersion . '/call/' . self::$identifierToToken[$identifier] . '/ring/' . $attendeeId);
 		$this->assertStatusCode($this->response, $statusCode);
 	}
 

tests/integration/features/callapi/notifications.feature

@@ -122,3 +122,35 @@ Feature: callapi/notifications
     Then user "participant2" has the following notifications
       | app | object_type | object_id | subject |
 
+  Scenario: Pinging a user that is removed gives 404
+    When user "participant1" creates room "room" (v4)
+      | roomType | 2 |
+      | roomName | room |
+    And user "participant1" adds user "participant2" to room "room" with 200 (v4)
+    Given user "participant1" joins room "room" with 200 (v4)
+    Given user "participant1" loads attendees attendee ids in room "room" (v4)
+    Given user "participant1" joins call "room" with 200 (v4)
+      | silent | true |
+    And user "participant1" removes user "participant2" from room "room" with 200 (v4)
+    Then user "participant2" has the following notifications
+      | app | object_type | object_id | subject |
+    Given user "participant1" pings user "participant2" to join call "room" with 404 (v4)
+    Then user "participant2" has the following notifications
+      | app | object_type | object_id | subject |
+
+  Scenario: Pinging a user that is removed gives 404
+    When user "participant1" creates room "room" (v4)
+      | roomType | 2 |
+      | roomName | room |
+    When user "participant2" creates room "room2" (v4)
+      | roomType | 2 |
+      | roomName | room2 |
+    Given user "participant1" joins room "room" with 200 (v4)
+    Given user "participant1" loads attendees attendee ids in room "room" (v4)
+    Given user "participant1" joins call "room" with 200 (v4)
+      | silent | true |
+    Then user "participant2" has the following notifications
+      | app | object_type | object_id | subject |
+    Given user "participant1" pings user "participant1" attendeeIdPlusOne to join call "room" with 404 (v4)
+    Then user "participant2" has the following notifications
+      | app | object_type | object_id | subject |