Skip to content

Commit

Permalink
Merge pull request #11978 from nextcloud/bugfix/11972/fix-empty-strin…
Browse files Browse the repository at this point in the history
…g-being-null

fix(federation): Fix empty strings being null in Oracle
  • Loading branch information
nickvergessen authored Apr 4, 2024
2 parents b319493 + 9782239 commit d931bc1
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 25 deletions.
2 changes: 1 addition & 1 deletion lib/Chat/MessageParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
6 changes: 3 additions & 3 deletions lib/Model/ProxyCacheMessage.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(),
Expand Down
34 changes: 25 additions & 9 deletions tests/integration/features/bootstrap/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down Expand Up @@ -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;
});
Expand All @@ -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();
Expand Down Expand Up @@ -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'];
}
Expand All @@ -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'];
}
}
Expand All @@ -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'];
Expand Down Expand Up @@ -494,7 +510,7 @@ private function assertRooms($rooms, TableNode $formData, bool $shouldOrder = fa
}

return $data;
}, $rooms, $formData->getHash()));
}, $rooms, $expected));
}

/**
Expand Down
24 changes: 12 additions & 12 deletions tests/integration/features/federation/chat.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit d931bc1

Please sign in to comment.