Merge pull request #11999 from nextcloud/backport/11978/stable29

[stable29] fix(federation): Fix empty strings being null in Oracle
nextcloud · Apr 4, 2024 · 3edc1c2 · 3edc1c2
2 parents 3a996a4 + 1c924c0
commit 3edc1c2
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 25 deletions.
diff --git a/lib/Chat/MessageParser.php b/lib/Chat/MessageParser.php
@@ -69,7 +69,7 @@ public function createMessageFromProxyCache(Room $room, ?Participant $participan
 		$message->setActor(
 			$proxy->getActorType(),
 			$proxy->getActorId(),
-			$proxy->getActorDisplayName(),
+			$proxy->getActorDisplayName() ?? '',
 		);
 
 		$message->setMessageType($proxy->getMessageType());

diff --git a/lib/Model/ProxyCacheMessage.php b/lib/Model/ProxyCacheMessage.php
@@ -42,8 +42,8 @@
  * @method string getActorType()
  * @method void setActorId(string $actorId)
  * @method string getActorId()
- * @method void setActorDisplayName(string $actorDisplayName)
- * @method string getActorDisplayName()
+ * @method void setActorDisplayName(?string $actorDisplayName)
+ * @method string|null getActorDisplayName()
  * @method void setMessageType(string $messageType)
  * @method string getMessageType()
  * @method void setSystemMessage(?string $systemMessage)
@@ -119,7 +119,7 @@ public function jsonSerialize(): array {
 		return [
 			'actorType' => $this->getActorType(),
 			'actorId' => $this->getActorId(),
-			'actorDisplayName' => $this->getActorDisplayName(),
+			'actorDisplayName' => $this->getActorDisplayName() ?? '',
 			'timestamp' => $this->getCreationDatetime()->getTimestamp(),
 			'expirationTimestamp' => $expirationTimestamp,
 			'messageType' => $this->getMessageType(),

diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php
@@ -303,12 +303,12 @@ public function userIsParticipantOfRooms(string $user, string $shouldOrder, stri
 		$rooms = $this->getDataFromResponse($this->response);
 
 		if ($shouldFilter === '') {
-			$rooms = array_filter($rooms, function ($room) {
+			$rooms = array_filter($rooms, static function (array $room) {
 				// Filter out "Talk updates" and "Note to self" conversations
 				return $room['type'] !== 4 && $room['type'] !== 6;
 			});
 		} elseif ($shouldFilter === 'note-to-self ') {
-			$rooms = array_filter($rooms, function ($room) {
+			$rooms = array_filter($rooms, static function (array $room) {
 				// Filter out "Talk updates" conversations
 				return $room['type'] !== 4;
 			});
@@ -343,7 +343,7 @@ public function userListsBreakoutRooms(string $user, string $identifier, int $st
 
 		$rooms = $this->getDataFromResponse($this->response);
 
-		$rooms = array_filter($rooms, function ($room) {
+		$rooms = array_filter($rooms, static function (array $room) {
 			// Filter out "Talk updates" and "Note to self" conversations
 			return $room['type'] !== 4 && $room['type'] !== 6;
 		});
@@ -360,7 +360,7 @@ public function userListsBreakoutRooms(string $user, string $identifier, int $st
 	 * @param array $rooms
 	 * @param TableNode $formData
 	 */
-	private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = false) {
+	private function assertRooms(array $rooms, TableNode $formData, bool $shouldOrder = false) {
 		Assert::assertCount(count($formData->getHash()), $rooms, 'Room count does not match');
 
 		$expected = $formData->getHash();
@@ -388,14 +388,26 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa
 					self::$identifierToId[$roomB['name']] = $idB;
 				}
 
+				if ($idA === $idB) {
+					if (isset($roomA['remoteServer'], $roomB['remoteServer'])) {
+						return $roomA['remoteServer'] < $roomB['remoteServer'] ? -1 : 1;
+					}
+					if (isset($roomA['remoteServer'])) {
+						return 1;
+					}
+					if (isset($roomB['remoteServer'])) {
+						return -1;
+					}
+				}
+
 				return $idA < $idB ? -1 : 1;
 			};
 
 			usort($rooms, $sorter);
 			usort($expected, $sorter);
 		}
 
-		Assert::assertEquals($expected, array_map(function ($room, $expectedRoom) {
+		Assert::assertEquals($expected, array_map(function (array $room, array $expectedRoom): array {
 			if (!isset(self::$identifierToToken[$room['name']])) {
 				self::$identifierToToken[$room['name']] = $room['token'];
 			}
@@ -411,7 +423,7 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa
 				$data['name'] = $room['name'];
 
 				// Breakout room regex
-				if (strpos($expectedRoom['name'], '/') === 0 && preg_match($expectedRoom['name'], $room['name'])) {
+				if (str_starts_with($expectedRoom['name'], '/') && preg_match($expectedRoom['name'], $room['name'])) {
 					$data['name'] = $expectedRoom['name'];
 				}
 			}
@@ -422,10 +434,14 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa
 				$data['type'] = (string) $room['type'];
 			}
 			if (isset($expectedRoom['remoteServer'])) {
-				$data['remoteServer'] = self::translateRemoteServer($room['remoteServer']);
+				$data['remoteServer'] = isset($room['remoteServer']) ? self::translateRemoteServer($room['remoteServer']) : '';
 			}
 			if (isset($expectedRoom['remoteToken'])) {
-				$data['remoteToken'] = self::$tokenToIdentifier[$room['remoteToken']] ?? 'unknown-token';
+				if (isset($room['remoteToken'])) {
+					$data['remoteToken'] = self::$tokenToIdentifier[$room['remoteToken']] ?? 'unknown-token';
+				} else {
+					$data['remoteToken'] = '';
+				}
 			}
 			if (isset($expectedRoom['hasPassword'])) {
 				$data['hasPassword'] = (string) $room['hasPassword'];
@@ -494,7 +510,7 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa
 			}
 
 			return $data;
-		}, $rooms, $formData->getHash()));
+		}, $rooms, $expected));
 	}
 
 	/**

diff --git a/tests/integration/features/federation/chat.feature b/tests/integration/features/federation/chat.feature
@@ -151,20 +151,20 @@ Feature: federation/chat
     And user "participant1" accepts invite to room "room" of server "LOCAL" with 200 (v1)
       | id   | name | type | remoteServer | remoteToken |
       | room | room | 2    | LOCAL        | room        |
-    Then user "participant1" is participant of the following rooms (v4)
-      | id   | type | lastMessage |
-      | room | 2    | {actor} invited you |
-      | room | 2    | {federated_user} accepted the invitation |
+    Then user "participant1" is participant of the following unordered rooms (v4)
+      | id   | name | type | remoteServer | remoteToken | lastMessage |
+      | room | room | 2    |              |             | {actor} invited you |
+      | room | room | 2    | LOCAL        | room        | {federated_user} accepted the invitation |
     And user "participant1" sends message "Message 1" to room "room" with 201
-    Then user "participant1" is participant of the following rooms (v4)
-      | id   | type | lastMessage | lastMessageActorType | lastMessageActorId |
-      | room | 2    | Message 1   | users                | participant1       |
-      | room | 2    | Message 1   | federated_users      | participant1@{$BASE_URL} |
+    Then user "participant1" is participant of the following unordered rooms (v4)
+      | id   | name | type | remoteServer | remoteToken | lastMessage | lastMessageActorType | lastMessageActorId |
+      | room | room | 2    |              |             | Message 1   | users                | participant1       |
+      | room | room | 2    | LOCAL        | room        | Message 1   | federated_users      | participant1@{$BASE_URL} |
     When user "participant1" sends reply "Message 1-1" on message "Message 1" to room "LOCAL::room" with 201
-    Then user "participant1" is participant of the following rooms (v4)
-      | id   | type | lastMessage | lastMessageActorType | lastMessageActorId |
-      | room | 2    | Message 1-1 | federated_users      | participant1@{$REMOTE_URL} |
-      | room | 2    | Message 1-1 | users                | participant1       |
+    Then user "participant1" is participant of the following unordered rooms (v4)
+      | id   | name | type | remoteServer | remoteToken | lastMessage | lastMessageActorType | lastMessageActorId |
+      | room | room | 2    |              |             | Message 1-1 | federated_users      | participant1@{$REMOTE_URL} |
+      | room | room | 2    | LOCAL        | room        | Message 1-1 | users                | participant1       |
 
   Scenario: Read marker checking
     Given the following "spreed" app config is set