From 58adc7fb369c01118bf84597d13b1a407bcc9201 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 28 Mar 2024 17:57:48 +0100 Subject: [PATCH 1/2] fix(federation): Fix empty strings being null in Oracle Signed-off-by: Joas Schilling --- lib/Chat/MessageParser.php | 2 +- lib/Model/ProxyCacheMessage.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Chat/MessageParser.php b/lib/Chat/MessageParser.php index de88ab506bc..9a504e3c377 100644 --- 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 index 095b48844c4..7e3e7dec5a6 100644 --- 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(), From 9782239ae71b64a687f6812877539f7c4c114ea5 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 3 Apr 2024 17:17:58 +0200 Subject: [PATCH 2/2] fix(tests): Fix sorting rooms with same name We can't sort by token here as the name-to-token map is overwritten as both rooms have the same name Signed-off-by: Joas Schilling --- .../features/bootstrap/FeatureContext.php | 34 ++++++++++++++----- .../features/federation/chat.feature | 24 ++++++------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index d8a18dd29c0..466022b3cb3 100644 --- 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,6 +388,18 @@ 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; }; @@ -395,7 +407,7 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa 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 index 82971dbeb2d..67b8160fb41 100644 --- 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