From cfdb5a5fb7d88dd1ccaf1b53e70db073fd1dbd10 Mon Sep 17 00:00:00 2001 From: Hamish Peebles Date: Tue, 12 Jan 2021 23:18:52 +0000 Subject: [PATCH 1/5] Switch to using classes to define Chat objects --- commands.txt | 2 +- .../public/actions/chats/createGroupChat.ts | 4 +- .../public/actions/chats/sendMessage.ts | 22 +- .../actions/chats/setupNewDirectChat.ts | 2 +- src/website/public/components/ChatList.tsx | 8 +- src/website/public/components/MainHeader.tsx | 1 + .../public/components/MessagesList.tsx | 3 +- src/website/public/model/chats.ts | 253 +++++++++++++++--- src/website/public/reducers/chatsReducer.ts | 190 +++---------- src/website/public/reducers/usersReducer.ts | 5 +- src/website/public/services/chats/getChats.ts | 46 ++-- 11 files changed, 306 insertions(+), 230 deletions(-) diff --git a/commands.txt b/commands.txt index 2973493084..ba3060bc1f 100644 --- a/commands.txt +++ b/commands.txt @@ -21,7 +21,7 @@ dfx canister call user_mgmt get_user_id '("Matt")' dfx canister call chats create_direct_chat '(principal "lpfg6-5goq7-vn6yz-wegiu-r6goc-sxoor-wj3b2-dpyup-k6i5m-qhv5m-xae", "Hi Matt, how is it going")' dfx canister call chats send_message '(45_292_552_032_833_753, "hello!!??")' dfx identity use matt -dfx canister call chats list_chats +dfx canister call chats get_chats '(record { updated_since = null; message_count_for_top_chat = null })' dfx canister call chats get_messages '(45_292_552_032_833_753, 1)' dfx canister call chats mark_read '(45_292_552_032_833_753, 2)' dfx canister call chats send_message '(45_292_552_032_833_753, "whats up?")' diff --git a/src/website/public/actions/chats/createGroupChat.ts b/src/website/public/actions/chats/createGroupChat.ts index 239d2795e5..2a5af4b8bd 100644 --- a/src/website/public/actions/chats/createGroupChat.ts +++ b/src/website/public/actions/chats/createGroupChat.ts @@ -1,7 +1,7 @@ import { Dispatch } from "react"; import chatsService from "../../services/chats/service"; -import { ChatId } from "../../model/chats"; +import { ChatId, NewGroupChat } from "../../model/chats"; import { UserId } from "../../model/users"; import { RootState } from "../../reducers"; @@ -52,7 +52,7 @@ export default function(subject: string, users: UserId[]) { // Messages may have been added on the UI before the chat was confirmed on the back end. These messages will // have been added to the 'chat.unconfirmedMessages' array. So we need to read the values out of this array, // then apply the state change to confirm the chat, then send those messages using the new chatId. - const messagesToSend = getState().chatsState.chats.find(c => c.kind === "newGroup" && c.id === tempId)!.unconfirmedMessages; + const messagesToSend = getState().chatsState.chats.find(c => c instanceof NewGroupChat && c.id === tempId)!.unconfirmedMessages; dispatch(outcomeEvent); diff --git a/src/website/public/actions/chats/sendMessage.ts b/src/website/public/actions/chats/sendMessage.ts index 77b73f2a2b..3916131d0e 100644 --- a/src/website/public/actions/chats/sendMessage.ts +++ b/src/website/public/actions/chats/sendMessage.ts @@ -2,7 +2,7 @@ import { Dispatch } from "react"; import chatsService from "../../services/chats/service"; import { SendDirectMessageResult } from "../../services/chats/sendDirectMessage"; -import { Chat, ChatId } from "../../model/chats"; +import { Chat, ChatId, DirectChat, GroupChat, NewDirectChat, NewGroupChat } from "../../model/chats"; import { Option } from "../../model/common"; import { UserId } from "../../model/users"; import { RootState } from "../../reducers"; @@ -12,18 +12,14 @@ export const SEND_MESSAGE_SUCCEEDED = "SEND_MESSAGE_SUCCEEDED"; export const SEND_MESSAGE_FAILED = "SEND_MESSAGE_FAILED"; export default function(chat: Chat, message: string) { - switch (chat.kind) { - case "direct": - return sendDirectMessage(chat.them, chat.chatId, message); - - case "group": - return sendGroupMessage(chat.chatId, message); - - case "newDirect": - return sendDirectMessage(chat.them, null, message); - - case "newGroup": - return sendMessageToNewGroup(chat.id, message); + if (chat instanceof DirectChat) { + return sendDirectMessage(chat.them, chat.chatId, message); + } else if (chat instanceof GroupChat) { + return sendGroupMessage(chat.chatId, message); + } else if (chat instanceof NewDirectChat) { + return sendDirectMessage(chat.them, null, message); + } else if (chat instanceof NewGroupChat) { + return sendMessageToNewGroup(chat.id, message); } } diff --git a/src/website/public/actions/chats/setupNewDirectChat.ts b/src/website/public/actions/chats/setupNewDirectChat.ts index 616a9591fe..0b7a1fabb6 100644 --- a/src/website/public/actions/chats/setupNewDirectChat.ts +++ b/src/website/public/actions/chats/setupNewDirectChat.ts @@ -68,7 +68,7 @@ export default function(username: string) { } function chatAlreadyExists(chats: Chat[], userId: UserId) : boolean { - const chat = chats.find(c => c.kind === "direct" && c.them === userId); + const chat = chats.find(c => "them" in c && c.them === userId); return Boolean(chat); } diff --git a/src/website/public/components/ChatList.tsx b/src/website/public/components/ChatList.tsx index cdb0a1c08b..4af50cbf05 100644 --- a/src/website/public/components/ChatList.tsx +++ b/src/website/public/components/ChatList.tsx @@ -1,8 +1,10 @@ import React from "react"; -import { Option } from "../model/common"; -import { RootState } from "../reducers"; import { useSelector } from "react-redux"; + +import { GroupChat } from "../model/chats"; +import { Option } from "../model/common"; import { UserId } from "../model/users"; +import { RootState } from "../reducers"; import ChatListItem from "./ChatListItem"; @@ -26,7 +28,7 @@ function ChatList() { userId = c.them; } else { name = c.subject; - key = c.kind === "group" ? "G-" + c.chatId.toString() : key = "NG-" + c.subject; + key = c instanceof GroupChat ? "G-" + c.chatId.toString() : key = "NG-" + c.subject; isGroup = true; userId = null; } diff --git a/src/website/public/components/MainHeader.tsx b/src/website/public/components/MainHeader.tsx index c8fd98a09a..6aeaadcee1 100644 --- a/src/website/public/components/MainHeader.tsx +++ b/src/website/public/components/MainHeader.tsx @@ -1,5 +1,6 @@ import React from "react"; import { useSelector } from "react-redux"; + import { RootState } from "../reducers"; import DefaultAvatar from "./defaultAvatar"; import GroupChatIcon from "../assets/icons/groupChatIcon.svg"; diff --git a/src/website/public/components/MessagesList.tsx b/src/website/public/components/MessagesList.tsx index 7afcf62d63..4948dcc6bc 100644 --- a/src/website/public/components/MessagesList.tsx +++ b/src/website/public/components/MessagesList.tsx @@ -2,6 +2,7 @@ import React from "react"; import { useSelector } from "react-redux"; import { RootState } from "../reducers"; +import { DirectChat } from "../model/chats"; import { Option } from "../model/common"; import { UserId } from "../model/users"; @@ -57,7 +58,7 @@ function MessagesList() { mergeWithPrevious }; children.push(); - } else if (chat.kind === "direct") { + } else if (chat instanceof DirectChat) { const props = { message: message.text, date: message.date, diff --git a/src/website/public/model/chats.ts b/src/website/public/model/chats.ts index 8c6f9b8714..99190fd413 100644 --- a/src/website/public/model/chats.ts +++ b/src/website/public/model/chats.ts @@ -1,45 +1,238 @@ -import { ConfirmedMessage, UnconfirmedMessage } from "./messages"; +import { ConfirmedMessage, RemoteMessage, UnconfirmedMessage } from "./messages"; import { UserId } from "./users"; - -export type ChatId = BigInt; +import * as setFunctions from "../utils/setFunctions"; export type Chat = ConfirmedChat | UnconfirmedChat; - export type ConfirmedChat = DirectChat | GroupChat; export type UnconfirmedChat = NewDirectChat | NewGroupChat; -export type DirectChat = ConfirmedChatCommon & { - kind: "direct", - them: UserId +export type ChatId = BigInt; + +abstract class ConfirmedChatBase { + chatId: ChatId; + updatedDate: Date; + readUpTo: number; + latestKnownMessageId: number; + confirmedMessages: ConfirmedMessage[]; + unconfirmedMessages: UnconfirmedMessage[]; + messagesToDownload: number[]; + messagesDownloading: number[]; + + protected constructor( + chatId: ChatId, + updatedDate: Date, + readUpTo: number, + latestKnownMessageId: number, + confirmedMessages: ConfirmedMessage[], + unconfirmedMessages: UnconfirmedMessage[]) { + this.chatId = chatId; + this.updatedDate = updatedDate; + this.readUpTo = readUpTo; + this.latestKnownMessageId = latestKnownMessageId; + this.confirmedMessages = confirmedMessages; + this.unconfirmedMessages = unconfirmedMessages; + this.messagesToDownload = []; + this.messagesDownloading = []; + } + + abstract clone() : ConfirmedChat; + + addMessage(message: ConfirmedMessage) { + this.addMessages([message]); + } + + addMessages(messages: ConfirmedMessage[], latestKnownMessageId?: number) : void { + // Ensure messages are sorted by id (they should be already so this should only do a single iteration) + messages.sort((a, b) => a.id - b.id); + + const chat = this; + const lowestCurrentMessageId = chat.confirmedMessages.length ? chat.confirmedMessages[0].id : null; + const lowestNewMessageId = messages[0].id; + + if (chat.messagesToDownload.length) { + messages + .filter(m => m.kind === "local") + .forEach(m => setFunctions.remove(chat.messagesToDownload, m.id)); + } + + let indexWhereNoLongerPrepending = 0; + if (lowestCurrentMessageId && lowestNewMessageId < lowestCurrentMessageId) { + // If we reach here, then we need to prepend at least 1 message to the current array + const shiftRequired = lowestCurrentMessageId - lowestNewMessageId; + const toPrepend: ConfirmedMessage[] = []; + for (let i = 0; i < messages.length && messages[i].id < lowestCurrentMessageId; i++) { + const message = messages[i]; + toPrepend[message.id - lowestCurrentMessageId + shiftRequired] = message; + indexWhereNoLongerPrepending++; + } + + // Check for gaps in the array of messages, if found, plug them with RemoteMessages and queue them for download + for (let id = lowestNewMessageId + 1; id < lowestCurrentMessageId; id++) { + const index = id - lowestNewMessageId; + if (!messages[index]) { + chat.confirmedMessages[index] = { + kind: "remote", + id: id + } as RemoteMessage; + + setFunctions.add(chat.messagesToDownload, id); + } + } + + chat.confirmedMessages.unshift(...toPrepend); + } + + const lowestMessageId = lowestCurrentMessageId + ? Math.min(lowestCurrentMessageId, lowestNewMessageId) + : lowestNewMessageId; + + // Now handle the later messages + for (let index = indexWhereNoLongerPrepending; index < messages.length; index++) { + const message = messages[index]; + const messageIndex = message.id - lowestMessageId; + + if (messageIndex < chat.confirmedMessages.length) { + // This is the only case where we overwrite an existing message, so first check if the existing message is + // 'local'. If it is we would be replacing it with a message that is the same or worse, so we do nothing. + if (chat.confirmedMessages[messageIndex].kind !== "local") { + chat.confirmedMessages[messageIndex] = message; + } + } else if (messageIndex === chat.confirmedMessages.length) { + chat.confirmedMessages.push(message); + } else { + // If we reach here then some messages are missing so we need to fill the gaps with RemoteMessages and mark + // them to be downloaded + const firstMissingMessageId = chat.confirmedMessages[chat.confirmedMessages.length - 1].id + 1; + const lastMissingMessageId = message.id - 1; + const indexToInsertAt = chat.confirmedMessages.length; + addMissingMessages(firstMissingMessageId, lastMissingMessageId, indexToInsertAt); + chat.confirmedMessages.push(message); + } + + if (message.kind === "local") { + if (chat.updatedDate < message.date) { + chat.updatedDate = message.date; + } + } + + if (chat.latestKnownMessageId < message.id) { + chat.latestKnownMessageId = message.id; + } + } + + // If after adding these messages the latestKnownMessageId value we have is still lower than what we got from the + // server then we need to add some missing messages and mark them to be downloaded. + if (latestKnownMessageId && chat.latestKnownMessageId < latestKnownMessageId) { + addMissingMessages(chat.latestKnownMessageId + 1, latestKnownMessageId, chat.latestKnownMessageId + 1); + chat.latestKnownMessageId = latestKnownMessageId; + } + + function addMissingMessages(fromId: number, toId: number, index: number) { + const missingMessages: RemoteMessage[] = []; + for (let id = fromId; id <= toId; id++) { + missingMessages.push({ kind: "remote", id }); + setFunctions.add(chat.messagesToDownload, id); + } + + chat.confirmedMessages.splice(index, 0, ...missingMessages); + } + } } -export type GroupChat = ConfirmedChatCommon & { - kind: "group", - subject: string, - participants: UserId[] +export class DirectChat extends ConfirmedChatBase { + them: UserId; + + constructor( + chatId: ChatId, + them: UserId, + updatedDate: Date, + readUpTo: number = 0, + latestKnownMessageId: number = 0, + confirmedMessages: ConfirmedMessage[] = [], + unconfirmedMessages: UnconfirmedMessage[] = []) { + super(chatId, updatedDate, readUpTo, latestKnownMessageId, confirmedMessages, unconfirmedMessages); + this.them = them; + } + + clone() : DirectChat { + return new DirectChat( + this.chatId, + this.them, + this.updatedDate, + this.readUpTo, + this.latestKnownMessageId, + this.confirmedMessages, + this.unconfirmedMessages); + } } -export type NewDirectChat = { - kind: "newDirect", - them: UserId, - unconfirmedMessages: UnconfirmedMessage[] +export class GroupChat extends ConfirmedChatBase { + subject: string; + participants: UserId[]; + + constructor( + chatId: ChatId, + subject: string, + participants: UserId[], + updatedDate: Date, + readUpTo: number = 0, + latestKnownMessageId: number = 0, + confirmedMessages: ConfirmedMessage[] = [], + unconfirmedMessages: UnconfirmedMessage[] = []) { + super(chatId, updatedDate, readUpTo, latestKnownMessageId, confirmedMessages, unconfirmedMessages); + this.subject = subject; + this.participants = participants; + } + + clone() : GroupChat { + return new GroupChat( + this.chatId, + this.subject, + this.participants, + this.updatedDate, + this.readUpTo, + this.latestKnownMessageId, + this.confirmedMessages, + this.unconfirmedMessages); + } } -export type NewGroupChat = { - kind: "newGroup", - id: Symbol, - subject: string, - participants: UserId[], - unconfirmedMessages: UnconfirmedMessage[] +abstract class UnconfirmedChatBase { + unconfirmedMessages: UnconfirmedMessage[]; + + protected constructor(unconfirmedMessages: UnconfirmedMessage[]) { + this.unconfirmedMessages = unconfirmedMessages; + } + + abstract clone() : UnconfirmedChat; +} + +export class NewDirectChat extends UnconfirmedChatBase { + them: UserId; + + constructor(them: UserId, unconfirmedMessages: UnconfirmedMessage[] = []) { + super(unconfirmedMessages); + this.them = them; + } + + clone(): NewDirectChat { + return new NewDirectChat(this.them, this.unconfirmedMessages); + } } -type ConfirmedChatCommon = { - chatId: ChatId, - updatedDate: Date, - readUpTo: number, - latestKnownMessageId: number, - messagesToDownload: number[], - messagesDownloading: number[], - confirmedMessages: ConfirmedMessage[], - unconfirmedMessages: UnconfirmedMessage[] +export class NewGroupChat extends UnconfirmedChatBase { + id: Symbol; + subject: string; + participants: UserId[]; + + constructor(id: Symbol, subject: string, participants: UserId[], unconfirmedMessages: UnconfirmedMessage[] = []) { + super(unconfirmedMessages); + this.id = id; + this.subject = subject; + this.participants = participants; + } + + clone(): NewGroupChat { + return new NewGroupChat(this.id, this.subject, this.participants, this.unconfirmedMessages); + } } diff --git a/src/website/public/reducers/chatsReducer.ts b/src/website/public/reducers/chatsReducer.ts index 05ce70122e..ba68105720 100644 --- a/src/website/public/reducers/chatsReducer.ts +++ b/src/website/public/reducers/chatsReducer.ts @@ -1,4 +1,13 @@ -import { Chat, ChatId, ConfirmedChat, DirectChat, GroupChat, NewDirectChat, NewGroupChat } from "../model/chats"; +import { + Chat, + ChatId, + ConfirmedChat, + DirectChat, + GroupChat, + NewDirectChat, + NewGroupChat, + UnconfirmedChat +} from "../model/chats"; import { Option, Timestamp } from "../model/common"; import { ConfirmedMessage, LocalMessage, Message, RemoteMessage, UnconfirmedMessage } from "../model/messages"; import { UserId } from "../model/users"; @@ -81,13 +90,10 @@ export default function(state: State = initialState, event: Event) : State { if (messagesIds.length) { chats = chats.slice(); - const chatCopy = { ...chat }; + const chatCopy = chat.clone(); chats[event.payload] = chatCopy; chatCopy.messagesToDownload = setFunctions.union(chatCopy.messagesToDownload, messagesIds); - addMessagesToChat(chatCopy, messagesIds.map(id => ({ - kind: "remote", - id - } as RemoteMessage))); + chatCopy.addMessages(messagesIds.map(id => ({ kind: "remote", id } as RemoteMessage))); } } @@ -101,13 +107,10 @@ export default function(state: State = initialState, event: Event) : State { case CREATE_GROUP_CHAT_REQUESTED: { const { tempId, subject, users } = event.payload; - const newChat: NewGroupChat = { - kind: "newGroup", - id: tempId, + const newChat: NewGroupChat = new NewGroupChat( + tempId, subject, - participants: users, - unconfirmedMessages: [] - }; + users); return { ...state, @@ -119,21 +122,13 @@ export default function(state: State = initialState, event: Event) : State { case CREATE_GROUP_CHAT_SUCCEEDED: { const { tempId, chatId, date } = event.payload; - const chatIndex = state.chats.findIndex(c => c.kind === "newGroup" && c.id === tempId); + const chatIndex = state.chats.findIndex(c => c instanceof NewGroupChat && c.id === tempId); const chat = state.chats[chatIndex] as NewGroupChat; - const newChat: GroupChat = { - kind: "group", - subject: chat.subject, - participants: chat.participants, + const newChat = new GroupChat( chatId, - updatedDate: date, - readUpTo: 0, - latestKnownMessageId: 0, - messagesToDownload: [], - messagesDownloading: [], - confirmedMessages: [], - unconfirmedMessages: [] - }; + chat.subject, + chat.participants, + date); const chatsCopy = state.chats.slice(); chatsCopy[chatIndex] = newChat; @@ -162,7 +157,7 @@ export default function(state: State = initialState, event: Event) : State { const { chatId, messageIds } = event.payload; const chatsCopy = state.chats.slice(); const chatIndex = findChatIndex(chatsCopy, chatId); - const chatCopy = { ...chatsCopy[chatIndex] } as ConfirmedChat; + const chatCopy = (chatsCopy[chatIndex] as ConfirmedChat).clone(); chatsCopy[chatIndex] = chatCopy; chatCopy.messagesDownloading = setFunctions.union(chatCopy.messagesDownloading, messageIds); @@ -177,7 +172,7 @@ export default function(state: State = initialState, event: Event) : State { const { request, result } = event.payload; const chatsCopy = state.chats.slice(); const chatIndex = findChatIndex(chatsCopy, request.chatId); - const chatCopy = { ...chatsCopy[chatIndex] } as ConfirmedChat; + const chatCopy = (chatsCopy[chatIndex] as ConfirmedChat).clone(); chatsCopy[chatIndex] = chatCopy; chatCopy.confirmedMessages = chatCopy.confirmedMessages.slice(); chatCopy.unconfirmedMessages = chatCopy.unconfirmedMessages.slice(); @@ -186,7 +181,7 @@ export default function(state: State = initialState, event: Event) : State { chatCopy.messagesToDownload = setFunctions.except(chatCopy.messagesToDownload, messageIds); chatCopy.messagesDownloading = setFunctions.except(chatCopy.messagesDownloading, request.messageIds); - addMessagesToChat(chatCopy, result.messages, result.latestMessageId); + chatCopy.addMessages(result.messages, result.latestMessageId); const selectedChatIndex = sortChatsAndReturnSelectedIndex(chatsCopy, state.selectedChatIndex!); @@ -201,7 +196,7 @@ export default function(state: State = initialState, event: Event) : State { const { chatId, messageIds } = event.payload; const chatsCopy = state.chats.slice(); const chatIndex = findChatIndex(chatsCopy, chatId); - const chatCopy = { ...chatsCopy[chatIndex] } as ConfirmedChat; + const chatCopy = (chatsCopy[chatIndex] as ConfirmedChat).clone(); chatsCopy[chatIndex] = chatCopy; chatCopy.messagesDownloading = setFunctions.except(chatCopy.messagesDownloading, messageIds); @@ -223,9 +218,9 @@ export default function(state: State = initialState, event: Event) : State { chats.forEach(c => { const chatIndex = findChatIndex(chatsCopy, c.chatId); if (chatIndex >= 0) { - const chatCopy = { ...chatsCopy[chatIndex] } as ConfirmedChat; + const chatCopy = (chatsCopy[chatIndex] as ConfirmedChat).clone(); chatsCopy[chatIndex] = chatCopy; - addMessagesToChat(chatCopy, c.confirmedMessages, c.latestKnownMessageId); + chatCopy.addMessages(c.confirmedMessages, c.latestKnownMessageId); } else { chatsCopy.push(c); } @@ -259,7 +254,7 @@ export default function(state: State = initialState, event: Event) : State { text: payload.message }; - const chatCopy = { ...chatsCopy[chatIndex] }; + const chatCopy = chatsCopy[chatIndex].clone(); chatCopy.unconfirmedMessages = [...chatCopy.unconfirmedMessages, unconfirmedMessage]; chatsCopy.splice(chatIndex, 1); @@ -282,22 +277,18 @@ export default function(state: State = initialState, event: Event) : State { // SEND_MESSAGE_SUCCEEDED will never happen on a NewGroupChat since messages need to be sent using either a // userId or a chatId and a NewGroupChat has neither. const chat = chatsCopy[chatIndex] as Exclude; - let chatCopy; - if (chat.kind === "newDirect") { - chatCopy = { - kind: "direct", - them: chat.them, - chatId: payload.chatId, - updatedDate: new Date(), - readUpTo: 0, - latestKnownMessageId: 0, - messagesToDownload: [], - messagesDownloading: [], - confirmedMessages: [], - unconfirmedMessages: chat.unconfirmedMessages - } as DirectChat; + let chatCopy: ConfirmedChat; + if (chat instanceof NewDirectChat) { + chatCopy = new DirectChat( + payload.chatId, + chat.them, + new Date(), + 0, + 0, + [], + chat.unconfirmedMessages); } else { - chatCopy = { ...chat }; + chatCopy = chat.clone(); chatCopy.messagesToDownload = chatCopy.messagesToDownload.slice(); chatCopy.confirmedMessages = chatCopy.confirmedMessages.slice(); chatCopy.unconfirmedMessages = chatCopy.unconfirmedMessages.slice(); @@ -320,8 +311,7 @@ export default function(state: State = initialState, event: Event) : State { chatCopy.unconfirmedMessages.splice(unconfirmedMessageIndex, 1); } - addMessageToChat(chatCopy, confirmedMessage); - setFunctions.remove(chatCopy.messagesToDownload, confirmedMessage.id); + chatCopy.addMessage(confirmedMessage); const selectedChatIndex = sortChatsAndReturnSelectedIndex(chatsCopy, state.selectedChatIndex!); @@ -335,11 +325,7 @@ export default function(state: State = initialState, event: Event) : State { case SETUP_NEW_DIRECT_CHAT_SUCCEEDED: { const { userId } = event.payload; - const newChat: NewDirectChat = { - kind: "newDirect", - them: userId, - unconfirmedMessages: [] - }; + const newChat: NewDirectChat = new NewDirectChat(userId); return { ...state, @@ -353,100 +339,6 @@ export default function(state: State = initialState, event: Event) : State { } } -function addMessageToChat(chat: ConfirmedChat, message: ConfirmedMessage) { - addMessagesToChat(chat, [message]); -} - -function addMessagesToChat(chat: ConfirmedChat, messages: ConfirmedMessage[], latestKnownMessageId?: Option) { - // Ensure messages are sorted by id (they should be already so this should only do a single iteration) - messages.sort((a, b) => a.id - b.id); - - const lowestCurrentMessageId = chat.confirmedMessages.length ? chat.confirmedMessages[0].id : null; - const lowestNewMessageId = messages[0].id; - - let indexWhereNoLongerPrepending = 0; - if (lowestCurrentMessageId && lowestNewMessageId < lowestCurrentMessageId) { - // If we reach here, then we need to prepend at least 1 message to the current array - const shiftRequired = lowestCurrentMessageId - lowestNewMessageId; - const toPrepend: ConfirmedMessage[] = []; - for (let i = 0; i < messages.length && messages[i].id < lowestCurrentMessageId; i++) { - const message = messages[i]; - toPrepend[message.id - lowestCurrentMessageId + shiftRequired] = message; - indexWhereNoLongerPrepending++; - } - - // Check for gaps in the array of messages, if found, plug them with RemoteMessages and queue them for download - for (let id = lowestNewMessageId + 1; id < lowestCurrentMessageId; id++) { - const index = id - lowestNewMessageId; - if (!messages[index]) { - chat.confirmedMessages[index] = { - kind: "remote", - id: id - } as RemoteMessage; - - setFunctions.add(chat.messagesToDownload, id); - } - } - - chat.confirmedMessages.unshift(...toPrepend); - } - - const lowestMessageId = lowestCurrentMessageId - ? Math.min(lowestCurrentMessageId, lowestNewMessageId) - : lowestNewMessageId; - - // Now handle the later messages - for (let index = indexWhereNoLongerPrepending; index < messages.length; index++) { - const message = messages[index]; - const messageIndex = message.id - lowestMessageId; - - if (messageIndex < chat.confirmedMessages.length) { - // This is the only case where we overwrite an existing message, so first check if the existing message is - // 'local'. If it is we would be replacing it with a message that is the same or worse, so we do nothing. - if (chat.confirmedMessages[messageIndex].kind !== "local") { - chat.confirmedMessages[messageIndex] = message; - } - } else if (messageIndex === chat.confirmedMessages.length) { - chat.confirmedMessages.push(message); - } else { - // If we reach here then some messages are missing so we need to fill the gaps with RemoteMessages and mark - // them to be downloaded - const firstMissingMessageId = chat.confirmedMessages[chat.confirmedMessages.length - 1].id + 1; - const lastMissingMessageId = message.id - 1; - const indexToInsertAt = chat.confirmedMessages.length; - addMissingMessages(firstMissingMessageId, lastMissingMessageId, indexToInsertAt); - chat.confirmedMessages.push(message); - } - - if (message.kind === "local") { - if (chat.updatedDate < message.date) { - chat.updatedDate = message.date; - } - } - - if (chat.latestKnownMessageId < message.id) { - chat.latestKnownMessageId = message.id; - } - } - - // If after adding these messages the latestKnownMessageId value we have is still lower than what we got from the - // server then we need to add some missing messages and mark them to be downloaded. - if (latestKnownMessageId && chat.latestKnownMessageId < latestKnownMessageId) { - addMissingMessages(chat.latestKnownMessageId + 1, latestKnownMessageId, chat.latestKnownMessageId + 1); - chat.latestKnownMessageId = latestKnownMessageId; - } - - function addMissingMessages(fromId: number, toId: number, index: number) { - const missingMessages: RemoteMessage[] = []; - for (let id = fromId; id <= toId; id++) { - missingMessages.push({ kind: "remote", id }); - setFunctions.add(chat.messagesToDownload, id); - } - - chat.confirmedMessages.splice(index, 0, ...missingMessages); - } -} - function getMessageIdsToFillLatestPage(messages: Message[], confirmedOnServerUpTo: number) : number[] { const minMessageIdRequired = Math.max(confirmedOnServerUpTo - PAGE_SIZE + 1, MIN_MESSAGE_ID); const maxMessageIdRequired = confirmedOnServerUpTo; @@ -501,5 +393,5 @@ function findDirectChatIndex(chats: Chat[], userId: UserId) : number { } function findNewGroupChatIndex(chats: Chat[], id: Symbol) : number { - return chats.findIndex(c => c.kind === "newGroup" && id === c.id); + return chats.findIndex(c => c instanceof NewGroupChat && id === c.id); } diff --git a/src/website/public/reducers/usersReducer.ts b/src/website/public/reducers/usersReducer.ts index a977d1fdbe..74c9933703 100644 --- a/src/website/public/reducers/usersReducer.ts +++ b/src/website/public/reducers/usersReducer.ts @@ -1,3 +1,4 @@ +import { DirectChat, GroupChat } from "../model/chats"; import { Option } from "../model/common"; import { UserId, UserSummary } from "../model/users"; @@ -65,12 +66,12 @@ export default function(state: State = initialState, event: Event) : State { const userDictionary: any = state.userDictionary; chats.forEach((c => { - if (c.kind === "direct") { + if (c instanceof DirectChat) { if (!userDictionary.hasOwnProperty(c.them.toString()) && !unknownUserIds.find(u => u === c.them)) { unknownUserIds.push(c.them); } - } else { + } else if (c instanceof GroupChat) { c.participants.forEach((p: UserId) => { if (!userDictionary.hasOwnProperty(p.toString()) && !unknownUserIds.find(u => u === p)) { diff --git a/src/website/public/services/chats/getChats.ts b/src/website/public/services/chats/getChats.ts index 495a9f7247..bac3b9d653 100644 --- a/src/website/public/services/chats/getChats.ts +++ b/src/website/public/services/chats/getChats.ts @@ -1,5 +1,5 @@ import canister from "ic:canisters/chats"; -import { ConfirmedChat, DirectChat, GroupChat } from "../../model/chats"; +import { Chat, DirectChat, GroupChat } from "../../model/chats"; import { Option, Timestamp } from "../../model/common"; import { fromCandid as chatIdFromCandid } from "../candidConverters/chatId"; import { fromCandid as localMessageFromCandid } from "../candidConverters/localMessage"; @@ -46,11 +46,11 @@ export type GetChatsResponse = export type Success = { kind: "success", - chats: ConfirmedChat[], + chats: Chat[], latestUpdateTimestamp: Option } -function convertToChat(value: any) : ConfirmedChat { +function convertToChat(value: any) : Chat { if (value.hasOwnProperty("Direct")) { return convertToDirectChat(value.Direct); } else if (value.hasOwnProperty("Group")) { @@ -62,36 +62,26 @@ function convertToChat(value: any) : ConfirmedChat { function convertToDirectChat(value: any) : DirectChat { const latestMessage = value.latest_messages[0]; - return { - kind: "direct", - them: userIdFromCandid(value.them), - chatId: chatIdFromCandid(value.id), - updatedDate: timestampToDate(value.updated_date), - readUpTo: latestMessage.id - value.unread, - latestKnownMessageId: latestMessage.id, - messagesToDownload: [], - messagesDownloading: [], - confirmedMessages: value.latest_messages.reverse().map(localMessageFromCandid), - unconfirmedMessages: [] - }; + return new DirectChat( + chatIdFromCandid(value.id), + userIdFromCandid(value.them), + timestampToDate(value.updated_date), + latestMessage.id - value.unread, + latestMessage.id, + value.latest_messages.reverse().map(localMessageFromCandid)); } function convertToGroupChat(value: any) : GroupChat { const latestMessageId = value.latest_messages.length > 0 ? value.latest_messages[0].id : 0; - return { - kind: "group", - chatId: chatIdFromCandid(value.id), - subject: value.subject, - updatedDate: timestampToDate(value.updated_date), - participants: value.participants.map(userIdFromCandid), - readUpTo: latestMessageId - value.unread, - latestKnownMessageId: latestMessageId, - messagesToDownload: [], - messagesDownloading: [], - confirmedMessages: value.latest_messages.reverse().map(localMessageFromCandid), - unconfirmedMessages: [] - }; + return new GroupChat( + chatIdFromCandid(value.id), + value.subject, + value.participants.map(userIdFromCandid), + timestampToDate(value.updated_date), + latestMessageId - value.unread, + latestMessageId, + value.latest_messages.reverse().map(localMessageFromCandid)); } From 2998a38ce817e9b857e534445755d596fe203f3f Mon Sep 17 00:00:00 2001 From: Hamish Peebles Date: Wed, 13 Jan 2021 14:55:39 +0000 Subject: [PATCH 2/5] Move message handling logic from chatsReducer to the new chat classes --- .../public/actions/chats/createGroupChat.ts | 4 +- .../public/actions/chats/sendMessage.ts | 60 ++--- src/website/public/components/ChatList.tsx | 11 +- .../public/components/MessagesList.tsx | 45 ++-- src/website/public/model/chats.ts | 255 ++++++++++-------- src/website/public/model/messages.ts | 1 - src/website/public/reducers/chatsReducer.ts | 82 ++---- src/website/public/services/chats/getChats.ts | 2 - 8 files changed, 217 insertions(+), 243 deletions(-) diff --git a/src/website/public/actions/chats/createGroupChat.ts b/src/website/public/actions/chats/createGroupChat.ts index 2a5af4b8bd..1fa322bd8b 100644 --- a/src/website/public/actions/chats/createGroupChat.ts +++ b/src/website/public/actions/chats/createGroupChat.ts @@ -2,6 +2,7 @@ import { Dispatch } from "react"; import chatsService from "../../services/chats/service"; import { ChatId, NewGroupChat } from "../../model/chats"; +import { UnconfirmedMessage } from "../../model/messages"; import { UserId } from "../../model/users"; import { RootState } from "../../reducers"; @@ -52,7 +53,8 @@ export default function(subject: string, users: UserId[]) { // Messages may have been added on the UI before the chat was confirmed on the back end. These messages will // have been added to the 'chat.unconfirmedMessages' array. So we need to read the values out of this array, // then apply the state change to confirm the chat, then send those messages using the new chatId. - const messagesToSend = getState().chatsState.chats.find(c => c instanceof NewGroupChat && c.id === tempId)!.unconfirmedMessages; + const chat = getState().chatsState.chats.find(c => c instanceof NewGroupChat && c.id === tempId)! + const messagesToSend = chat.messages as UnconfirmedMessage[]; dispatch(outcomeEvent); diff --git a/src/website/public/actions/chats/sendMessage.ts b/src/website/public/actions/chats/sendMessage.ts index 3916131d0e..951ee779b5 100644 --- a/src/website/public/actions/chats/sendMessage.ts +++ b/src/website/public/actions/chats/sendMessage.ts @@ -4,6 +4,7 @@ import chatsService from "../../services/chats/service"; import { SendDirectMessageResult } from "../../services/chats/sendDirectMessage"; import { Chat, ChatId, DirectChat, GroupChat, NewDirectChat, NewGroupChat } from "../../model/chats"; import { Option } from "../../model/common"; +import { LocalMessage } from "../../model/messages"; import { UserId } from "../../model/users"; import { RootState } from "../../reducers"; @@ -25,16 +26,13 @@ export default function(chat: Chat, message: string) { function sendDirectMessage(userId: UserId, chatId: Option, message: string) { return async (dispatch: Dispatch, getState: () => RootState) => { - const id = Symbol("id"); - const requestEvent: SendMessageRequestedEvent = { type: SEND_MESSAGE_REQUESTED, payload: { kind: "direct", userId: userId, chatId: chatId, - message: message, - unconfirmedMessageId: id + message: message } }; @@ -54,11 +52,13 @@ function sendDirectMessage(userId: UserId, chatId: Option, message: stri kind: "direct", userId: userId, chatId: chatId ?? (response.result as SendDirectMessageResult).chatId, - sender: myUserId, - message: message, - unconfirmedMessageId: id, - confirmedMessageId: response.result.messageId, - confirmedMessageDate: response.result.date + message: { + kind: "local", + id: response.result.messageId, + date: response.result.date, + sender: myUserId, + text: message + } } } as SendMessageSucceededEvent; } else { @@ -73,15 +73,12 @@ function sendDirectMessage(userId: UserId, chatId: Option, message: stri function sendGroupMessage(chatId: ChatId, message: string) { return async (dispatch: Dispatch, getState: () => RootState) => { - const id = Symbol("id"); - const requestEvent: SendMessageRequestedEvent = { type: SEND_MESSAGE_REQUESTED, payload: { kind: "group", chatId: chatId, - message: message, - unconfirmedMessageId: id + message: message } }; @@ -98,11 +95,13 @@ function sendGroupMessage(chatId: ChatId, message: string) { payload: { kind: "group", chatId: chatId, - sender: myUserId, - message: message, - unconfirmedMessageId: id, - confirmedMessageId: response.result.messageId, - confirmedMessageDate: response.result.date + message: { + kind: "local", + id: response.result.messageId, + date: response.result.date, + sender: myUserId, + text: message + } } } as SendMessageSucceededEvent; } else { @@ -149,15 +148,13 @@ export type SendDirectMessageRequest = { kind: "direct", userId: UserId, chatId: Option, - message: string, - unconfirmedMessageId: Symbol + message: string } export type SendGroupMessageRequest = { kind: "group", chatId: ChatId, - message: string, - unconfirmedMessageId: Symbol + message: string } export type SendMessageToNewGroupRequest = { @@ -168,20 +165,15 @@ export type SendMessageToNewGroupRequest = { export type SendMessageSuccess = SendDirectMessageSuccess | SendGroupMessageSuccess; -export type SendDirectMessageSuccess = SendMessageSuccessCommon & { +export type SendDirectMessageSuccess = { kind: "direct", - userId: UserId -} - -export type SendGroupMessageSuccess = SendMessageSuccessCommon & { - kind: "group" + userId: UserId, + chatId: ChatId, + message: LocalMessage } -type SendMessageSuccessCommon = { +export type SendGroupMessageSuccess = { + kind: "group", chatId: ChatId, - sender: UserId, - message: string, - unconfirmedMessageId: Symbol, - confirmedMessageId: number, - confirmedMessageDate: Date + message: LocalMessage } diff --git a/src/website/public/components/ChatList.tsx b/src/website/public/components/ChatList.tsx index 4af50cbf05..984ded20d2 100644 --- a/src/website/public/components/ChatList.tsx +++ b/src/website/public/components/ChatList.tsx @@ -34,12 +34,11 @@ function ChatList() { } let latestMessageText = ""; - if (c.unconfirmedMessages.length) { - latestMessageText = c.unconfirmedMessages[c.unconfirmedMessages.length - 1].text; - } else if ("confirmedMessages" in c && c.confirmedMessages.length) { - const latestMessage = c.confirmedMessages[c.confirmedMessages.length - 1]; - if ("text" in latestMessage) { - latestMessageText = latestMessage.text; + for (let i = c.messages.length - 1; i >= 0; i--) { + const message = c.messages[i]; + if ("text" in message) { + latestMessageText = message.text; + break; } } diff --git a/src/website/public/components/MessagesList.tsx b/src/website/public/components/MessagesList.tsx index 4948dcc6bc..8cd26a86d1 100644 --- a/src/website/public/components/MessagesList.tsx +++ b/src/website/public/components/MessagesList.tsx @@ -32,13 +32,26 @@ function MessagesList() { let lastSeenDate: Option = null; let lastSeenDayString: Option = null; let prevMessageSender: Option = null; - if ("confirmedMessages" in chat) { - for (let i = 0; i < chat.confirmedMessages.length; i++) { - const message = chat.confirmedMessages[i]; - if (message.kind === "remote") { - continue; - } + for (let i = 0; i < chat.messages.length; i++) { + const message = chat.messages[i]; + if (message.kind === "remote") { + continue; + } else if (message.kind === "unconfirmed") { + const now = new Date(); + const mergeWithPrevious: boolean = + lastSeenDate !== null && + (prevMessageSender === null || prevMessageSender === myUserId) && + now.getTime() - lastSeenDate.getTime() < MERGE_MESSAGES_SENT_BY_SAME_USER_WITHIN_MILLIS; + + const props = { + message: message.text, + mergeWithPrevious + }; + children.push(); + prevMessageSender = myUserId; + lastSeenDate = now; + } else { const dayString = message.date.toDateString(); if (lastSeenDayString === null || lastSeenDayString !== dayString) { children.push(); @@ -82,26 +95,6 @@ function MessagesList() { } } - for (let i = 0; i < chat.unconfirmedMessages.length; i++) { - const message = chat.unconfirmedMessages[i]; - - const now = new Date(); - - const mergeWithPrevious: boolean = - lastSeenDate !== null && - (prevMessageSender === null || prevMessageSender === myUserId) && - now.getTime() - lastSeenDate.getTime() < MERGE_MESSAGES_SENT_BY_SAME_USER_WITHIN_MILLIS; - - const props = { - message: message.text, - mergeWithPrevious - }; - children.push(); - - lastSeenDate = now; - prevMessageSender = myUserId; - } - return (
{children} diff --git a/src/website/public/model/chats.ts b/src/website/public/model/chats.ts index 99190fd413..9aaeb63000 100644 --- a/src/website/public/model/chats.ts +++ b/src/website/public/model/chats.ts @@ -1,4 +1,5 @@ -import { ConfirmedMessage, RemoteMessage, UnconfirmedMessage } from "./messages"; +import { Option } from "./common"; +import { LocalMessage, Message, RemoteMessage, UnconfirmedMessage } from "./messages"; import { UserId } from "./users"; import * as setFunctions from "../utils/setFunctions"; @@ -12,130 +13,173 @@ abstract class ConfirmedChatBase { chatId: ChatId; updatedDate: Date; readUpTo: number; - latestKnownMessageId: number; - confirmedMessages: ConfirmedMessage[]; - unconfirmedMessages: UnconfirmedMessage[]; + messages: Message[]; messagesToDownload: number[]; messagesDownloading: number[]; + #earliestConfirmedMessageId: Option; + #latestConfirmedMessageId: Option; + #minimumUnconfirmedMessageIndex: number; protected constructor( chatId: ChatId, updatedDate: Date, readUpTo: number, - latestKnownMessageId: number, - confirmedMessages: ConfirmedMessage[], - unconfirmedMessages: UnconfirmedMessage[]) { + messages: Message[]) { this.chatId = chatId; this.updatedDate = updatedDate; this.readUpTo = readUpTo; - this.latestKnownMessageId = latestKnownMessageId; - this.confirmedMessages = confirmedMessages; - this.unconfirmedMessages = unconfirmedMessages; + this.messages = messages; this.messagesToDownload = []; this.messagesDownloading = []; + this.#earliestConfirmedMessageId = this.calculateEarliestConfirmedMessageId(); + this.#latestConfirmedMessageId = this.calculateLatestConfirmedMessageId(); + this.#minimumUnconfirmedMessageIndex = 0; } abstract clone() : ConfirmedChat; - addMessage(message: ConfirmedMessage) { + addMessage = (message: LocalMessage) : void => { this.addMessages([message]); } - addMessages(messages: ConfirmedMessage[], latestKnownMessageId?: number) : void { + addMessages = (messages: LocalMessage[]) : void => { // Ensure messages are sorted by id (they should be already so this should only do a single iteration) messages.sort((a, b) => a.id - b.id); - const chat = this; - const lowestCurrentMessageId = chat.confirmedMessages.length ? chat.confirmedMessages[0].id : null; - const lowestNewMessageId = messages[0].id; + // These 2 setters will ensure the messages array covers the full range of ids from the new messages + this.earliestConfirmedMessageId = messages[0].id; + this.latestConfirmedMessageId = messages[messages.length - 1].id; - if (chat.messagesToDownload.length) { - messages - .filter(m => m.kind === "local") - .forEach(m => setFunctions.remove(chat.messagesToDownload, m.id)); - } + for (let index = 0; index < messages.length; index++) { + const message = messages[index]; + const messageIndex = this.getMessageIndex(message.id); - let indexWhereNoLongerPrepending = 0; - if (lowestCurrentMessageId && lowestNewMessageId < lowestCurrentMessageId) { - // If we reach here, then we need to prepend at least 1 message to the current array - const shiftRequired = lowestCurrentMessageId - lowestNewMessageId; - const toPrepend: ConfirmedMessage[] = []; - for (let i = 0; i < messages.length && messages[i].id < lowestCurrentMessageId; i++) { - const message = messages[i]; - toPrepend[message.id - lowestCurrentMessageId + shiftRequired] = message; - indexWhereNoLongerPrepending++; + const currentMessage = this.messages[messageIndex]; + if (currentMessage.kind === "local") { + // If the current message is 'local' then this message has already been added + continue; } + this.messages[messageIndex] = message; - // Check for gaps in the array of messages, if found, plug them with RemoteMessages and queue them for download - for (let id = lowestNewMessageId + 1; id < lowestCurrentMessageId; id++) { - const index = id - lowestNewMessageId; - if (!messages[index]) { - chat.confirmedMessages[index] = { - kind: "remote", - id: id - } as RemoteMessage; - - setFunctions.add(chat.messagesToDownload, id); - } - } + this.removeMatchingUnconfirmedMessage(message.text); - chat.confirmedMessages.unshift(...toPrepend); + if (this.updatedDate < message.date) { + this.updatedDate = message.date; + } } - const lowestMessageId = lowestCurrentMessageId - ? Math.min(lowestCurrentMessageId, lowestNewMessageId) - : lowestNewMessageId; + this.queueMissingMessagesForDownload(); + } - // Now handle the later messages - for (let index = indexWhereNoLongerPrepending; index < messages.length; index++) { - const message = messages[index]; - const messageIndex = message.id - lowestMessageId; - - if (messageIndex < chat.confirmedMessages.length) { - // This is the only case where we overwrite an existing message, so first check if the existing message is - // 'local'. If it is we would be replacing it with a message that is the same or worse, so we do nothing. - if (chat.confirmedMessages[messageIndex].kind !== "local") { - chat.confirmedMessages[messageIndex] = message; - } - } else if (messageIndex === chat.confirmedMessages.length) { - chat.confirmedMessages.push(message); - } else { - // If we reach here then some messages are missing so we need to fill the gaps with RemoteMessages and mark - // them to be downloaded - const firstMissingMessageId = chat.confirmedMessages[chat.confirmedMessages.length - 1].id + 1; - const lastMissingMessageId = message.id - 1; - const indexToInsertAt = chat.confirmedMessages.length; - addMissingMessages(firstMissingMessageId, lastMissingMessageId, indexToInsertAt); - chat.confirmedMessages.push(message); + addUnconfirmedMessage = (message: string) : void => { + this.messages.push({ + kind: "unconfirmed", + text: message + } as UnconfirmedMessage); + } + + get earliestConfirmedMessageId() : number { + return this.#earliestConfirmedMessageId ?? 0; + } + + set earliestConfirmedMessageId(value: number) { + if (!this.#earliestConfirmedMessageId) { + this.messages.splice(0, 0, { kind: "remote", id: value }); + this.#latestConfirmedMessageId = value; + } else if (value >= this.#earliestConfirmedMessageId) { + return; + } else { + const toPrepend: RemoteMessage[] = []; + for (let id = value; id < this.#earliestConfirmedMessageId; id++) { + toPrepend.push({kind: "remote", id}); } + this.messages.splice(0, 0, ...toPrepend); + } + this.#earliestConfirmedMessageId = value; + } - if (message.kind === "local") { - if (chat.updatedDate < message.date) { - chat.updatedDate = message.date; - } + get latestConfirmedMessageId() : number { + return this.#latestConfirmedMessageId ?? 0; + } + + set latestConfirmedMessageId(value: number) { + if (!this.#latestConfirmedMessageId) { + this.messages.splice(0, 0, { kind: "remote", id: value }); + this.#earliestConfirmedMessageId = value; + } else if (value <= this.#latestConfirmedMessageId) { + return; + } else { + const toAdd: RemoteMessage[] = []; + for (let id = this.#latestConfirmedMessageId + 1; id <= value; id++) { + toAdd.push({ kind: "remote", id }); } + this.messages.splice(this.getMessageIndex(this.#latestConfirmedMessageId + 1), 0, ...toAdd); + } + this.#latestConfirmedMessageId = value; + } - if (chat.latestKnownMessageId < message.id) { - chat.latestKnownMessageId = message.id; + removeMatchingUnconfirmedMessage(text: string) { + let indexOfMatch: number = -1; + for (let index = this.#minimumUnconfirmedMessageIndex; index < this.messages.length; index++) { + const message = this.messages[index]; + if (message.kind !== "unconfirmed") { + this.#minimumUnconfirmedMessageIndex = index; + } else if (message.text === text) { + indexOfMatch = index; + break; } } - // If after adding these messages the latestKnownMessageId value we have is still lower than what we got from the - // server then we need to add some missing messages and mark them to be downloaded. - if (latestKnownMessageId && chat.latestKnownMessageId < latestKnownMessageId) { - addMissingMessages(chat.latestKnownMessageId + 1, latestKnownMessageId, chat.latestKnownMessageId + 1); - chat.latestKnownMessageId = latestKnownMessageId; + if (indexOfMatch >= 0) { + this.messages.splice(indexOfMatch, 1); } + } + + queueMissingMessagesForDownload = () : void => { + const missingMessages = this.messages.filter(m => m.kind === "remote").map(m => (m as RemoteMessage).id); + setFunctions.unionWith(this.messagesToDownload, missingMessages); + } + + calculateEarliestConfirmedMessageId = () : Option => { + return this.messages.length && this.messages[0].kind !== "unconfirmed" + ? this.messages[0].id + : null; + } + + calculateLatestConfirmedMessageId = () : Option => { + for (let index = this.messages.length - 1; index >= 0; index--) { + const message = this.messages[index]; + if (message.kind !== "unconfirmed") { + return message.id; + } + } + return null; + } - function addMissingMessages(fromId: number, toId: number, index: number) { - const missingMessages: RemoteMessage[] = []; - for (let id = fromId; id <= toId; id++) { - missingMessages.push({ kind: "remote", id }); - setFunctions.add(chat.messagesToDownload, id); + calculateLowestUnconfirmedExpectedMessageId = (startingFromId: number) : Option => { + const startingIndex = Math.max(this.getMessageIndex(startingFromId), 0); + for (let index = startingIndex; index < this.messages.length; index++) { + const message = this.messages[index]; + if (message.kind === "unconfirmed") { + return this.getMessageIdFromIndex(index); } + } + return null; + } + + getMessageIndex = (messageId: number) : number => { + const lowestMessageId = this.messages.length && this.messages[0].kind !== "unconfirmed" + ? this.messages[0].id + : messageId; - chat.confirmedMessages.splice(index, 0, ...missingMessages); + return messageId - lowestMessageId; + } + + getMessageIdFromIndex = (index: number) : number => { + if (!this.#earliestConfirmedMessageId) { + return 0; } + return this.#earliestConfirmedMessageId + index; } } @@ -147,10 +191,8 @@ export class DirectChat extends ConfirmedChatBase { them: UserId, updatedDate: Date, readUpTo: number = 0, - latestKnownMessageId: number = 0, - confirmedMessages: ConfirmedMessage[] = [], - unconfirmedMessages: UnconfirmedMessage[] = []) { - super(chatId, updatedDate, readUpTo, latestKnownMessageId, confirmedMessages, unconfirmedMessages); + messages: Message[] = []) { + super(chatId, updatedDate, readUpTo, messages); this.them = them; } @@ -160,9 +202,7 @@ export class DirectChat extends ConfirmedChatBase { this.them, this.updatedDate, this.readUpTo, - this.latestKnownMessageId, - this.confirmedMessages, - this.unconfirmedMessages); + this.messages); } } @@ -176,10 +216,8 @@ export class GroupChat extends ConfirmedChatBase { participants: UserId[], updatedDate: Date, readUpTo: number = 0, - latestKnownMessageId: number = 0, - confirmedMessages: ConfirmedMessage[] = [], - unconfirmedMessages: UnconfirmedMessage[] = []) { - super(chatId, updatedDate, readUpTo, latestKnownMessageId, confirmedMessages, unconfirmedMessages); + messages: Message[] = []) { + super(chatId, updatedDate, readUpTo, messages); this.subject = subject; this.participants = participants; } @@ -191,32 +229,37 @@ export class GroupChat extends ConfirmedChatBase { this.participants, this.updatedDate, this.readUpTo, - this.latestKnownMessageId, - this.confirmedMessages, - this.unconfirmedMessages); + this.messages); } } abstract class UnconfirmedChatBase { - unconfirmedMessages: UnconfirmedMessage[]; + messages: UnconfirmedMessage[]; - protected constructor(unconfirmedMessages: UnconfirmedMessage[]) { - this.unconfirmedMessages = unconfirmedMessages; + protected constructor(messages: UnconfirmedMessage[]) { + this.messages = messages; } abstract clone() : UnconfirmedChat; + + addUnconfirmedMessage = (message: string) => { + this.messages.push({ + kind: "unconfirmed", + text: message + } as UnconfirmedMessage); + } } export class NewDirectChat extends UnconfirmedChatBase { them: UserId; - constructor(them: UserId, unconfirmedMessages: UnconfirmedMessage[] = []) { - super(unconfirmedMessages); + constructor(them: UserId, messages: UnconfirmedMessage[] = []) { + super(messages); this.them = them; } clone(): NewDirectChat { - return new NewDirectChat(this.them, this.unconfirmedMessages); + return new NewDirectChat(this.them, this.messages); } } @@ -225,14 +268,14 @@ export class NewGroupChat extends UnconfirmedChatBase { subject: string; participants: UserId[]; - constructor(id: Symbol, subject: string, participants: UserId[], unconfirmedMessages: UnconfirmedMessage[] = []) { - super(unconfirmedMessages); + constructor(id: Symbol, subject: string, participants: UserId[], messages: UnconfirmedMessage[] = []) { + super(messages); this.id = id; this.subject = subject; this.participants = participants; } clone(): NewGroupChat { - return new NewGroupChat(this.id, this.subject, this.participants, this.unconfirmedMessages); + return new NewGroupChat(this.id, this.subject, this.participants, this.messages); } } diff --git a/src/website/public/model/messages.ts b/src/website/public/model/messages.ts index df20f12901..bab1c96509 100644 --- a/src/website/public/model/messages.ts +++ b/src/website/public/model/messages.ts @@ -19,6 +19,5 @@ export type RemoteMessage = { export type UnconfirmedMessage = { kind: "unconfirmed", - id: Symbol, text: string } diff --git a/src/website/public/reducers/chatsReducer.ts b/src/website/public/reducers/chatsReducer.ts index ba68105720..d7265c2d66 100644 --- a/src/website/public/reducers/chatsReducer.ts +++ b/src/website/public/reducers/chatsReducer.ts @@ -86,15 +86,11 @@ export default function(state: State = initialState, event: Event) : State { const chat = state.chats[event.payload]; let chats = state.chats; if ("chatId" in chat) { - const messagesIds = getMessageIdsToFillLatestPage(chat.confirmedMessages, chat.latestKnownMessageId); - - if (messagesIds.length) { - chats = chats.slice(); - const chatCopy = chat.clone(); - chats[event.payload] = chatCopy; - chatCopy.messagesToDownload = setFunctions.union(chatCopy.messagesToDownload, messagesIds); - chatCopy.addMessages(messagesIds.map(id => ({ kind: "remote", id } as RemoteMessage))); - } + const chatCopy = chat.clone(); + chatCopy.messages = chatCopy.messages.slice(); + chatCopy.messagesToDownload = chatCopy.messagesToDownload.slice(); + chatCopy.earliestConfirmedMessageId = chatCopy.latestConfirmedMessageId - PAGE_SIZE; + chatCopy.queueMissingMessagesForDownload(); } return { @@ -174,14 +170,10 @@ export default function(state: State = initialState, event: Event) : State { const chatIndex = findChatIndex(chatsCopy, request.chatId); const chatCopy = (chatsCopy[chatIndex] as ConfirmedChat).clone(); chatsCopy[chatIndex] = chatCopy; - chatCopy.confirmedMessages = chatCopy.confirmedMessages.slice(); - chatCopy.unconfirmedMessages = chatCopy.unconfirmedMessages.slice(); - - const messageIds = result.messages.map((m: LocalMessage) => m.id); - chatCopy.messagesToDownload = setFunctions.except(chatCopy.messagesToDownload, messageIds); + chatCopy.messages = chatCopy.messages.slice(); chatCopy.messagesDownloading = setFunctions.except(chatCopy.messagesDownloading, request.messageIds); - chatCopy.addMessages(result.messages, result.latestMessageId); + chatCopy.addMessages(result.messages); const selectedChatIndex = sortChatsAndReturnSelectedIndex(chatsCopy, state.selectedChatIndex!); @@ -220,7 +212,9 @@ export default function(state: State = initialState, event: Event) : State { if (chatIndex >= 0) { const chatCopy = (chatsCopy[chatIndex] as ConfirmedChat).clone(); chatsCopy[chatIndex] = chatCopy; - chatCopy.addMessages(c.confirmedMessages, c.latestKnownMessageId); + // These messages have just come from the server so are all of type LocalMessage + const messages = c.messages as LocalMessage[]; + chatCopy.addMessages(messages); } else { chatsCopy.push(c); } @@ -248,14 +242,8 @@ export default function(state: State = initialState, event: Event) : State { chatIndex = findNewGroupChatIndex(chatsCopy, payload.unconfirmedChatId); } - const unconfirmedMessage : UnconfirmedMessage = { - kind: "unconfirmed", - id: "unconfirmedMessageId" in payload ? payload.unconfirmedMessageId : Symbol("id"), - text: payload.message - }; - const chatCopy = chatsCopy[chatIndex].clone(); - chatCopy.unconfirmedMessages = [...chatCopy.unconfirmedMessages, unconfirmedMessage]; + chatCopy.addUnconfirmedMessage(payload.message); chatsCopy.splice(chatIndex, 1); chatsCopy.unshift(chatCopy); @@ -282,36 +270,18 @@ export default function(state: State = initialState, event: Event) : State { chatCopy = new DirectChat( payload.chatId, chat.them, - new Date(), + payload.message.date, 0, - 0, - [], - chat.unconfirmedMessages); + chat.messages); } else { chatCopy = chat.clone(); chatCopy.messagesToDownload = chatCopy.messagesToDownload.slice(); - chatCopy.confirmedMessages = chatCopy.confirmedMessages.slice(); - chatCopy.unconfirmedMessages = chatCopy.unconfirmedMessages.slice(); + chatCopy.messages = chatCopy.messages.slice(); } chatsCopy[chatIndex] = chatCopy; - const confirmedMessage: LocalMessage = { - kind: "local", - id: payload.confirmedMessageId, - date: payload.confirmedMessageDate, - sender: payload.sender, - text: payload.message - }; - - const unconfirmedMessageIndex = chatCopy.unconfirmedMessages.findIndex(m => - m.kind === "unconfirmed" && m.id === payload.unconfirmedMessageId); - - if (unconfirmedMessageIndex >= 0) { - chatCopy.unconfirmedMessages.splice(unconfirmedMessageIndex, 1); - } - - chatCopy.addMessage(confirmedMessage); + chatCopy.addMessage(payload.message); const selectedChatIndex = sortChatsAndReturnSelectedIndex(chatsCopy, state.selectedChatIndex!); @@ -339,28 +309,6 @@ export default function(state: State = initialState, event: Event) : State { } } -function getMessageIdsToFillLatestPage(messages: Message[], confirmedOnServerUpTo: number) : number[] { - const minMessageIdRequired = Math.max(confirmedOnServerUpTo - PAGE_SIZE + 1, MIN_MESSAGE_ID); - const maxMessageIdRequired = confirmedOnServerUpTo; - const requiredMessageIds = []; - - if (messages.length && messages[0].kind !== "unconfirmed") { - const firstMessageId = messages[0].id; - for (let id = minMessageIdRequired; id <= maxMessageIdRequired; id++) { - const index = id - firstMessageId; - if (index < 0 || index >= messages.length || messages[index].kind !== "local") { - requiredMessageIds.push(id); - } - } - } else { - for (let id = minMessageIdRequired; id <= maxMessageIdRequired; id++) { - requiredMessageIds.push(id); - } - } - - return requiredMessageIds; -} - function sortChatsAndReturnSelectedIndex(chats: Chat[], selectedIndex: Option) { const selectedChat = selectedIndex !== null ? chats[selectedIndex] : null; chats.sort((a, b) => { diff --git a/src/website/public/services/chats/getChats.ts b/src/website/public/services/chats/getChats.ts index bac3b9d653..e47b4f155d 100644 --- a/src/website/public/services/chats/getChats.ts +++ b/src/website/public/services/chats/getChats.ts @@ -67,7 +67,6 @@ function convertToDirectChat(value: any) : DirectChat { userIdFromCandid(value.them), timestampToDate(value.updated_date), latestMessage.id - value.unread, - latestMessage.id, value.latest_messages.reverse().map(localMessageFromCandid)); } @@ -80,7 +79,6 @@ function convertToGroupChat(value: any) : GroupChat value.participants.map(userIdFromCandid), timestampToDate(value.updated_date), latestMessageId - value.unread, - latestMessageId, value.latest_messages.reverse().map(localMessageFromCandid)); } From d2234990417cc4299e1b9f6bf55d235f25ee0d33 Mon Sep 17 00:00:00 2001 From: Hamish Peebles Date: Wed, 13 Jan 2021 15:14:39 +0000 Subject: [PATCH 3/5] Tighten constraint --- src/website/public/services/chats/getChats.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/website/public/services/chats/getChats.ts b/src/website/public/services/chats/getChats.ts index e47b4f155d..ab755cf933 100644 --- a/src/website/public/services/chats/getChats.ts +++ b/src/website/public/services/chats/getChats.ts @@ -1,5 +1,5 @@ import canister from "ic:canisters/chats"; -import { Chat, DirectChat, GroupChat } from "../../model/chats"; +import { ConfirmedChat, DirectChat, GroupChat } from "../../model/chats"; import { Option, Timestamp } from "../../model/common"; import { fromCandid as chatIdFromCandid } from "../candidConverters/chatId"; import { fromCandid as localMessageFromCandid } from "../candidConverters/localMessage"; @@ -46,11 +46,11 @@ export type GetChatsResponse = export type Success = { kind: "success", - chats: Chat[], + chats: ConfirmedChat[], latestUpdateTimestamp: Option } -function convertToChat(value: any) : Chat { +function convertToChat(value: any) : ConfirmedChat { if (value.hasOwnProperty("Direct")) { return convertToDirectChat(value.Direct); } else if (value.hasOwnProperty("Group")) { From 0b00a86e9ac49864496089249d786edf721da1ec Mon Sep 17 00:00:00 2001 From: Hamish Peebles Date: Wed, 13 Jan 2021 15:34:20 +0000 Subject: [PATCH 4/5] Set additional fields in ctor --- src/website/public/model/chats.ts | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/website/public/model/chats.ts b/src/website/public/model/chats.ts index 9aaeb63000..9949637bed 100644 --- a/src/website/public/model/chats.ts +++ b/src/website/public/model/chats.ts @@ -24,13 +24,15 @@ abstract class ConfirmedChatBase { chatId: ChatId, updatedDate: Date, readUpTo: number, - messages: Message[]) { + messages: Message[], + messagesToDownload: number[] = [], + messagesDownloading: number[] = []) { this.chatId = chatId; this.updatedDate = updatedDate; this.readUpTo = readUpTo; this.messages = messages; - this.messagesToDownload = []; - this.messagesDownloading = []; + this.messagesToDownload = messagesToDownload; + this.messagesDownloading = messagesDownloading; this.#earliestConfirmedMessageId = this.calculateEarliestConfirmedMessageId(); this.#latestConfirmedMessageId = this.calculateLatestConfirmedMessageId(); this.#minimumUnconfirmedMessageIndex = 0; @@ -191,8 +193,10 @@ export class DirectChat extends ConfirmedChatBase { them: UserId, updatedDate: Date, readUpTo: number = 0, - messages: Message[] = []) { - super(chatId, updatedDate, readUpTo, messages); + messages: Message[] = [], + messagesToDownload: number[] = [], + messagesDownloading: number[] = []) { + super(chatId, updatedDate, readUpTo, messages, messagesToDownload, messagesDownloading); this.them = them; } @@ -202,7 +206,9 @@ export class DirectChat extends ConfirmedChatBase { this.them, this.updatedDate, this.readUpTo, - this.messages); + this.messages, + this.messagesToDownload, + this.messagesDownloading); } } @@ -216,8 +222,10 @@ export class GroupChat extends ConfirmedChatBase { participants: UserId[], updatedDate: Date, readUpTo: number = 0, - messages: Message[] = []) { - super(chatId, updatedDate, readUpTo, messages); + messages: Message[] = [], + messagesToDownload: number[] = [], + messagesDownloading: number[] = []) { + super(chatId, updatedDate, readUpTo, messages, messagesToDownload, messagesDownloading); this.subject = subject; this.participants = participants; } @@ -229,7 +237,9 @@ export class GroupChat extends ConfirmedChatBase { this.participants, this.updatedDate, this.readUpTo, - this.messages); + this.messages, + this.messagesToDownload, + this.messagesDownloading); } } From a03ef67aefd67f5b5d52b43957e6f98268aa148d Mon Sep 17 00:00:00 2001 From: Hamish Peebles Date: Wed, 13 Jan 2021 15:50:27 +0000 Subject: [PATCH 5/5] Small code cleanup --- src/website/public/model/chats.ts | 40 ++++----------------- src/website/public/reducers/chatsReducer.ts | 9 +++-- 2 files changed, 13 insertions(+), 36 deletions(-) diff --git a/src/website/public/model/chats.ts b/src/website/public/model/chats.ts index 9949637bed..a305b80ba0 100644 --- a/src/website/public/model/chats.ts +++ b/src/website/public/model/chats.ts @@ -80,8 +80,9 @@ abstract class ConfirmedChatBase { } as UnconfirmedMessage); } - get earliestConfirmedMessageId() : number { - return this.#earliestConfirmedMessageId ?? 0; + queueMissingMessagesForDownload = () : void => { + const missingMessages = this.messages.filter(m => m.kind === "remote").map(m => (m as RemoteMessage).id); + setFunctions.unionWith(this.messagesToDownload, missingMessages); } set earliestConfirmedMessageId(value: number) { @@ -100,10 +101,6 @@ abstract class ConfirmedChatBase { this.#earliestConfirmedMessageId = value; } - get latestConfirmedMessageId() : number { - return this.#latestConfirmedMessageId ?? 0; - } - set latestConfirmedMessageId(value: number) { if (!this.#latestConfirmedMessageId) { this.messages.splice(0, 0, { kind: "remote", id: value }); @@ -120,7 +117,7 @@ abstract class ConfirmedChatBase { this.#latestConfirmedMessageId = value; } - removeMatchingUnconfirmedMessage(text: string) { + private removeMatchingUnconfirmedMessage(text: string) { let indexOfMatch: number = -1; for (let index = this.#minimumUnconfirmedMessageIndex; index < this.messages.length; index++) { const message = this.messages[index]; @@ -137,18 +134,13 @@ abstract class ConfirmedChatBase { } } - queueMissingMessagesForDownload = () : void => { - const missingMessages = this.messages.filter(m => m.kind === "remote").map(m => (m as RemoteMessage).id); - setFunctions.unionWith(this.messagesToDownload, missingMessages); - } - - calculateEarliestConfirmedMessageId = () : Option => { + private calculateEarliestConfirmedMessageId = () : Option => { return this.messages.length && this.messages[0].kind !== "unconfirmed" ? this.messages[0].id : null; } - calculateLatestConfirmedMessageId = () : Option => { + private calculateLatestConfirmedMessageId = () : Option => { for (let index = this.messages.length - 1; index >= 0; index--) { const message = this.messages[index]; if (message.kind !== "unconfirmed") { @@ -158,31 +150,13 @@ abstract class ConfirmedChatBase { return null; } - calculateLowestUnconfirmedExpectedMessageId = (startingFromId: number) : Option => { - const startingIndex = Math.max(this.getMessageIndex(startingFromId), 0); - for (let index = startingIndex; index < this.messages.length; index++) { - const message = this.messages[index]; - if (message.kind === "unconfirmed") { - return this.getMessageIdFromIndex(index); - } - } - return null; - } - - getMessageIndex = (messageId: number) : number => { + private getMessageIndex = (messageId: number) : number => { const lowestMessageId = this.messages.length && this.messages[0].kind !== "unconfirmed" ? this.messages[0].id : messageId; return messageId - lowestMessageId; } - - getMessageIdFromIndex = (index: number) : number => { - if (!this.#earliestConfirmedMessageId) { - return 0; - } - return this.#earliestConfirmedMessageId + index; - } } export class DirectChat extends ConfirmedChatBase { diff --git a/src/website/public/reducers/chatsReducer.ts b/src/website/public/reducers/chatsReducer.ts index d7265c2d66..f799999a93 100644 --- a/src/website/public/reducers/chatsReducer.ts +++ b/src/website/public/reducers/chatsReducer.ts @@ -83,10 +83,13 @@ type Event = export default function(state: State = initialState, event: Event) : State { switch (event.type) { case CHAT_SELECTED: { - const chat = state.chats[event.payload]; + const selectedChatIndex = event.payload; + let chat = state.chats[selectedChatIndex]; let chats = state.chats; if ("chatId" in chat) { const chatCopy = chat.clone(); + chats = chats.slice(); + chats[selectedChatIndex] = chatCopy; chatCopy.messages = chatCopy.messages.slice(); chatCopy.messagesToDownload = chatCopy.messagesToDownload.slice(); chatCopy.earliestConfirmedMessageId = chatCopy.latestConfirmedMessageId - PAGE_SIZE; @@ -95,8 +98,8 @@ export default function(state: State = initialState, event: Event) : State { return { ...state, - chats: chats, - selectedChatIndex: event.payload + chats, + selectedChatIndex }; }