borogove
created pr with
borogove/selfChat-avatar.1
cmds
checkout latest patchset:
ssh pr.pico.sh pull borogove:selfChat-avatar | git am -3checkout specific patchset revision:
ssh pr.pico.sh pull borogove:selfChat-avatar [rev] | git am -3add changes to patch request:
git format-patch main --stdout | ssh pr.pico.sh borogove:selfChat-avataradd comment to patch request:
ssh pr.pico.sh comment borogove:selfChat-avatar "lgtm!"
Patchset
borogove/selfChat-avatar.1
Do not overwrite selfChat avatar with null
Stephen Paul Weber
2026-10-21T18:20:33ZDuring startOffline we hydrate all the chats before assigning any of them into the chats array. However, this means that when a SerializedChat of a Channel has toChat called on it, it will call new Channel, which will call client.getDirectChat(client.accountId()). This sees no such chat, and makes a new one and stores it, overwriting the one we already had with one that, among other things, now has no avatar. The fix is to get the selfChat into chats/chatsIndex first before we call toChat on anyone else.
Semantic diff summary
0 added,
0 modified,
0 signature changed,
0 removed
across 0 analyzed files
(2 files skipped: unsupported file type)
+23
-11
borogove/Client.hx
#
| ... | ... | @@ -875,20 +875,32 @@ class Client extends EventEmitter { | |
| 875 | 875 | ||
| 876 | 876 | return persistence.getChats(accountId()); | |
| 877 | 877 | }).then((protoChats) -> { | |
| 878 | - | protoChats.sort((a, b) -> a.chatId == accountId() ? 1 : 0); | |
| 879 | 878 | var oneProtoChat = null; | |
| 880 | 879 | final chatPromises = []; | |
| 881 | - | while ((oneProtoChat = protoChats.pop()) != null) { | |
| 882 | - | chatPromises.push(oneProtoChat.toChat(this, stream, persistence)); | |
| 883 | - | } | |
| 884 | - | return thenshim.PromiseTools.all(chatPromises).then(theChats -> { | |
| 885 | - | chats = theChats; | |
| 886 | - | for (chat in chats) { | |
| 887 | - | chatsIndex[chat.chatId] = chat; | |
| 880 | + | ||
| 881 | + | final selfChatIndex = protoChats.findIndex(a -> a.chatId == accountId()); | |
| 882 | + | return (if (selfChatIndex == -1) { | |
| 883 | + | Promise.resolve(null); | |
| 884 | + | } else { | |
| 885 | + | protoChats.splice(selfChatIndex, 1)[0].toChat(this, stream, persistence).then(selfChat -> { | |
| 886 | + | chats = [selfChat]; | |
| 887 | + | chatsIndex[selfChat.chatId] = selfChat; | |
| 888 | + | chatPromises.push(Promise.resolve(selfChat)); | |
| 889 | + | return null; | |
| 890 | + | }); | |
| 891 | + | }).then(_ -> { | |
| 892 | + | while ((oneProtoChat = protoChats.pop()) != null) { | |
| 893 | + | chatPromises.push(oneProtoChat.toChat(this, stream, persistence)); | |
| 888 | 894 | } | |
| 889 | - | getDirectChat(accountId()); // Ensure self chat exists | |
| 890 | - | getDirectChat(JID.parse(accountId()).domain); // Ensure server chat exists | |
| 891 | - | return persistence.getChatsUnreadDetails(accountId(), chats); | |
| 895 | + | return thenshim.PromiseTools.all(chatPromises).then(theChats -> { | |
| 896 | + | chats = theChats; | |
| 897 | + | for (chat in chats) { | |
| 898 | + | chatsIndex[chat.chatId] = chat; | |
| 899 | + | } | |
| 900 | + | getDirectChat(accountId()); // Ensure self chat exists | |
| 901 | + | getDirectChat(JID.parse(accountId()).domain); // Ensure server chat exists | |
| 902 | + | return persistence.getChatsUnreadDetails(accountId(), chats); | |
| 903 | + | }); | |
| 892 | 904 | }); | |
| 893 | 905 | }).then((details) -> { | |
| 894 | 906 | for (detail in details) { |
+94
-0
test/TestClient.hx
#
| ... | ... | @@ -638,6 +638,79 @@ class TestClient extends utest.Test { | |
| 638 | 638 | client.start(); | |
| 639 | 639 | } | |
| 640 | 640 | ||
| 641 | + | public function testRestoreChannelDoesNotOverrideSelfChat(async: Async) { | |
| 642 | + | final accountId = "me@example.com"; | |
| 643 | + | final channelId = "room@example.com"; | |
| 644 | + | final avatarBytes = haxe.io.Bytes.ofHex("0102030405060708090a0b0c0d0e0f1011121314").getData(); | |
| 645 | + | ||
| 646 | + | final selfProtoChat = new SerializedChat( | |
| 647 | + | accountId, | |
| 648 | + | true, | |
| 649 | + | true, | |
| 650 | + | avatarBytes, | |
| 651 | + | new Map(), | |
| 652 | + | null, | |
| 653 | + | "Self Name", | |
| 654 | + | Open, | |
| 655 | + | false, | |
| 656 | + | new Status("", ""), | |
| 657 | + | null, | |
| 658 | + | null, | |
| 659 | + | null, | |
| 660 | + | null, | |
| 661 | + | false, | |
| 662 | + | false, | |
| 663 | + | new borogove.Chat.StringMapNullableKey(), | |
| 664 | + | null, | |
| 665 | + | null, | |
| 666 | + | [], | |
| 667 | + | "DirectChat" | |
| 668 | + | ); | |
| 669 | + | ||
| 670 | + | final channelProtoChat = new SerializedChat( | |
| 671 | + | channelId, | |
| 672 | + | false, | |
| 673 | + | false, | |
| 674 | + | null, | |
| 675 | + | new Map(), | |
| 676 | + | null, | |
| 677 | + | "Channel Name", | |
| 678 | + | Open, | |
| 679 | + | false, | |
| 680 | + | new Status("", ""), | |
| 681 | + | null, | |
| 682 | + | null, | |
| 683 | + | null, | |
| 684 | + | null, | |
| 685 | + | false, | |
| 686 | + | false, | |
| 687 | + | new borogove.Chat.StringMapNullableKey(), | |
| 688 | + | null, | |
| 689 | + | null, | |
| 690 | + | [], | |
| 691 | + | "Channel" | |
| 692 | + | ); | |
| 693 | + | ||
| 694 | + | final persistence = new RestoreChatsMockPersistence([channelProtoChat, selfProtoChat]); | |
| 695 | + | final client = new Client(accountId, persistence); | |
| 696 | + | ||
| 697 | + | client.startOffline().then(_ -> { | |
| 698 | + | final storedSelfChats = persistence.storedChats.filter(c -> c.chatId == accountId); | |
| 699 | + | Assert.equals(0, storedSelfChats.length); | |
| 700 | + | ||
| 701 | + | final selfChat = client.getDirectChat(accountId, false); | |
| 702 | + | Assert.notNull(selfChat); | |
| 703 | + | Assert.equals("Self Name", selfChat.displayName); | |
| 704 | + | Assert.equals(selfProtoChat.avatarSha1, selfChat.avatarSha1); | |
| 705 | + | ||
| 706 | + | final channel = Std.downcast(client.getChat(channelId), borogove.Chat.Channel); | |
| 707 | + | Assert.notNull(channel); | |
| 708 | + | Assert.equals(selfChat.getPhoto(), channel.self.photoUri); | |
| 709 | + | ||
| 710 | + | async.done(); | |
| 711 | + | }); | |
| 712 | + | } | |
| 713 | + | ||
| 641 | 714 | public function testGetChannelsReturnsOnlyChannels() { | |
| 642 | 715 | final persistence = new Dummy(); | |
| 643 | 716 | final client = new Client("test@example.com", persistence); |
| ... | ... | @@ -1326,3 +1399,24 @@ class VoiceRequestMockPersistence extends Dummy { | |
| 1326 | 1399 | return Promise.resolve(true); | |
| 1327 | 1400 | } | |
| 1328 | 1401 | } | |
| 1402 | + | ||
| 1403 | + | @:access(borogove) | |
| 1404 | + | class RestoreChatsMockPersistence extends Dummy { | |
| 1405 | + | public var storedChats: Array<Chat> = []; | |
| 1406 | + | public var chats: Array<SerializedChat>; | |
| 1407 | + | ||
| 1408 | + | public function new(chats: Array<SerializedChat>) { | |
| 1409 | + | super(); | |
| 1410 | + | this.chats = chats; | |
| 1411 | + | } | |
| 1412 | + | ||
| 1413 | + | override public function getChats(accountId: String): Promise<Array<SerializedChat>> { | |
| 1414 | + | return Promise.resolve(chats.copy()); | |
| 1415 | + | } | |
| 1416 | + | ||
| 1417 | + | override public function storeChats(accountId: String, chats: Array<Chat>) { | |
| 1418 | + | for (c in chats) { | |
| 1419 | + | storedChats.push(c); | |
| 1420 | + | } | |
| 1421 | + | } | |
| 1422 | + | } |