From 34ff9c6aa2fe5c1e2ac7ad0514b253f52a1fdba5 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Fri, 29 Nov 2024 17:56:40 +0000 Subject: [PATCH] core: change direction to orderBy for history, use enum As already done for Swift/Kotlin, this change renames the direction parameter of the history parameters to orderBy. Instead of using string values, we now use an enum. The ChatApi now does a mapping to the backend format (which itself will change at a later date). --- README.md | 2 +- src/core/chat-api.ts | 26 ++++++++++++++++---- src/core/index.ts | 1 + src/core/messages.ts | 33 +++++++++++++++++++------- src/react/README.md | 4 +++- src/react/hooks/use-messages.ts | 2 +- test/core/messages.integration.test.ts | 15 ++++++------ test/core/messages.test.ts | 22 ++++++++--------- 8 files changed, 71 insertions(+), 34 deletions(-) diff --git a/README.md b/README.md index e8c2c213..9e477a5f 100644 --- a/README.md +++ b/README.md @@ -386,7 +386,7 @@ The messages object also exposes the `get` method which can be used to request h to the given criteria. It returns a paginated response that can be used to request more messages. ```typescript -const historicalMessages = await room.messages.get({ direction: 'backwards', limit: 50 }); +const historicalMessages = await room.messages.get({ orderBy: ResultOrder.NewestFirst, limit: 50 }); console.log(historicalMessages.items); if (historicalMessages.hasNext()) { const next = await historicalMessages.next(); diff --git a/src/core/chat-api.ts b/src/core/chat-api.ts index ec340b49..3a79e976 100644 --- a/src/core/chat-api.ts +++ b/src/core/chat-api.ts @@ -2,13 +2,14 @@ import * as Ably from 'ably'; import { Logger } from './logger.js'; import { DefaultMessage, Message, MessageActionMetadata, MessageHeaders, MessageMetadata } from './message.js'; +import { ResultOrder } from './messages.js'; import { OccupancyEvent } from './occupancy.js'; import { PaginatedResult } from './query.js'; export interface GetMessagesQueryParams { start?: number; end?: number; - direction?: 'forwards' | 'backwards'; + orderBy?: ResultOrder; limit?: number; /** * Serial indicating the starting point for message retrieval. @@ -20,6 +21,14 @@ export interface GetMessagesQueryParams { fromSerial?: string; } +/** + * In the REST API, we currently use the `direction` query parameter to specify the order of messages instead + * of orderBy. So define this type for conversion purposes. + */ +type ApiGetMessagesQueryParams = Omit & { + direction?: 'forwards' | 'backwards'; +}; + export interface CreateMessageResponse { serial: string; createdAt: number; @@ -96,9 +105,18 @@ export class ChatApi { async getMessages(roomId: string, params: GetMessagesQueryParams): Promise> { roomId = encodeURIComponent(roomId); - return this._makeAuthorizedPaginatedRequest(`/chat/v2/rooms/${roomId}/messages`, params).then((data) => { - return this._recursivePaginateMessages(data); - }); + + // convert the params into internal format + const apiParams: ApiGetMessagesQueryParams = { ...params }; + if (params.orderBy) { + apiParams.direction = params.orderBy === ResultOrder.OldestFirst ? 'forwards' : 'backwards'; + } + + return this._makeAuthorizedPaginatedRequest(`/chat/v2/rooms/${roomId}/messages`, apiParams).then( + (data) => { + return this._recursivePaginateMessages(data); + }, + ); } private _recursivePaginateMessages(data: PaginatedResult): PaginatedResult { diff --git a/src/core/index.ts b/src/core/index.ts index 48167fa9..636a1003 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -42,6 +42,7 @@ export type { Messages, MessageSubscriptionResponse, QueryOptions, + ResultOrder, SendMessageParams, UpdateMessageParams, } from './messages.js'; diff --git a/src/core/messages.ts b/src/core/messages.ts index 7632560e..995bb12f 100644 --- a/src/core/messages.ts +++ b/src/core/messages.ts @@ -39,6 +39,21 @@ const MessageActionsToEventsMap: Map = new Ma [ChatMessageActions.MessageDelete, MessageEvents.Deleted], ]); +/** + * The order in which results should be returned when performing a paginated query (e.g. message history). + */ +export enum ResultOrder { + /** + * Return results in ascending order (oldest first). + */ + OldestFirst = 'oldestFirst', + + /** + * Return results in descending order (newest first). + */ + NewestFirst = 'newestFirst', +} + /** * Options for querying messages in a chat room. */ @@ -68,13 +83,13 @@ export interface QueryOptions { /** * The direction to query messages in. - * If `forwards`, the response will include messages from the start of the time window to the end. - * If `backwards`, the response will include messages from the end of the time window to the start. - * If not provided, the default is `forwards`. + * If {@link ResultOrder.OldestFirst}, the response will include messages from the start of the time window to the end. + * If {@link ResultOrder.NewestFirst}, the response will include messages from the end of the time window to the start. + * If not provided, the default is {@link ResultOrder.OldestFirst}. * - * @defaultValue forwards + * @defaultValue {@link ResultOrder.OldestFirst} */ - direction?: 'forwards' | 'backwards'; + orderBy?: ResultOrder; } /** @@ -186,7 +201,7 @@ export interface MessageSubscriptionResponse { * @param params Options for the history query. * @returns A promise that resolves with the paginated result of messages, in newest-to-oldest order. */ - getPreviousMessages(params: Omit): Promise>; + getPreviousMessages(params: Omit): Promise>; } /** @@ -351,7 +366,7 @@ export class DefaultMessages */ private async _getBeforeSubscriptionStart( listener: MessageListener, - params: Omit, + params: Omit, ): Promise> { this._logger.trace(`DefaultSubscriptionManager.getBeforeSubscriptionStart();`); @@ -374,7 +389,7 @@ export class DefaultMessages // Query messages from the subscription point to the start of the time window return this._chatApi.getMessages(this._roomId, { ...params, - direction: 'backwards', + orderBy: ResultOrder.NewestFirst, ...subscriptionPointParams, }); } @@ -584,7 +599,7 @@ export class DefaultMessages this._logger.trace('Messages.unsubscribe();'); super.off(listener); }, - getPreviousMessages: (params: Omit) => + getPreviousMessages: (params: Omit) => this._getBeforeSubscriptionStart(listener, params), }; } diff --git a/src/react/README.md b/src/react/README.md index 8ec81aca..51395b37 100644 --- a/src/react/README.md +++ b/src/react/README.md @@ -225,7 +225,9 @@ const MyComponent = () => { const [message, setMessage] = useState(); const handleGetMessages = () => { // fetch the last 3 messages, oldest to newest - get({ limit: 3, direction: 'forwards' }).then((result) => console.log('Previous messages: ', result.items)); + get({ limit: 3, orderBy: ResultOrder.oldestFirst }).then((result) => + console.log('Previous messages: ', result.items), + ); }; const handleMessageSend = () => { diff --git a/src/react/hooks/use-messages.ts b/src/react/hooks/use-messages.ts index 7412f67b..5b216309 100644 --- a/src/react/hooks/use-messages.ts +++ b/src/react/hooks/use-messages.ts @@ -145,7 +145,7 @@ export const useMessages = (params?: UseMessagesParams): UseMessagesResponse => return; } - return (params: Omit) => { + return (params: Omit) => { // If we've unmounted, then the subscription is gone and we can't call getPreviousMessages // So return a dummy object that should be thrown away anyway logger.debug('useMessages(); getPreviousMessages called', { roomId: context.roomId }); diff --git a/test/core/messages.integration.test.ts b/test/core/messages.integration.test.ts index 4a786ba6..2cd9841b 100644 --- a/test/core/messages.integration.test.ts +++ b/test/core/messages.integration.test.ts @@ -5,6 +5,7 @@ import { beforeEach, describe, expect, it } from 'vitest'; import { ChatClient } from '../../src/core/chat.ts'; import { MessageEvents } from '../../src/core/events.ts'; import { Message } from '../../src/core/message.ts'; +import { ResultOrder } from '../../src/core/messages.ts'; import { RealtimeChannelWithOptions } from '../../src/core/realtime-extensions.ts'; import { RoomOptionsDefaults } from '../../src/core/room-options.ts'; import { RoomStatus } from '../../src/core/room-status.ts'; @@ -218,7 +219,7 @@ describe('messages integration', () => { const message3 = await room.messages.send({ text: 'You underestimate my power!' }); // Do a history request to get all 3 messages - const history = await room.messages.get({ limit: 3, direction: 'forwards' }); + const history = await room.messages.get({ limit: 3, orderBy: ResultOrder.OldestFirst }); expect(history.items).toEqual([ expect.objectContaining({ @@ -255,7 +256,7 @@ describe('messages integration', () => { const deletedMessage1 = await room.messages.delete(message1, { description: 'Deleted message' }); // Do a history request to get the deleted message - const history = await room.messages.get({ limit: 3, direction: 'forwards' }); + const history = await room.messages.get({ limit: 3, orderBy: ResultOrder.OldestFirst }); expect(history.items).toEqual([ expect.objectContaining({ @@ -288,7 +289,7 @@ describe('messages integration', () => { ); // Do a history request to get the update message - const history = await room.messages.get({ limit: 3, direction: 'forwards' }); + const history = await room.messages.get({ limit: 3, orderBy: ResultOrder.OldestFirst }); expect(history.items).toEqual([ expect.objectContaining({ @@ -318,7 +319,7 @@ describe('messages integration', () => { const message4 = await room.messages.send({ text: "Don't try it!" }); // Do a history request to get the first 3 messages - const history1 = await room.messages.get({ limit: 3, direction: 'forwards' }); + const history1 = await room.messages.get({ limit: 3, orderBy: ResultOrder.OldestFirst }); expect(history1.items).toEqual([ expect.objectContaining({ @@ -423,7 +424,7 @@ describe('messages integration', () => { const message4 = await room.messages.send({ text: "Don't try it!" }); // Do a history request to get the first 3 messages - const history1 = await room.messages.get({ limit: 3, direction: 'forwards' }); + const history1 = await room.messages.get({ limit: 3, orderBy: ResultOrder.OldestFirst }); expect(history1.items).toEqual([ expect.objectContaining({ @@ -473,7 +474,7 @@ describe('messages integration', () => { const message4 = await room.messages.send({ text: "Don't try it!" }); // Do a history request to get the last 3 messages - const history1 = await room.messages.get({ limit: 3, direction: 'backwards' }); + const history1 = await room.messages.get({ limit: 3, orderBy: ResultOrder.NewestFirst }); expect(history1.items).toEqual([ expect.objectContaining({ @@ -561,7 +562,7 @@ describe('messages integration', () => { expect.objectContaining(expectedMessages[1]), ]); - const history = await room.messages.get({ limit: 2, direction: 'forwards' }); + const history = await room.messages.get({ limit: 2, orderBy: ResultOrder.OldestFirst }); expect(history.items.length).toEqual(2); expect(history.items, 'history messages to match').toEqual([ diff --git a/test/core/messages.test.ts b/test/core/messages.test.ts index e28e7073..fc8f34e2 100644 --- a/test/core/messages.test.ts +++ b/test/core/messages.test.ts @@ -5,7 +5,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ChatApi, GetMessagesQueryParams } from '../../src/core/chat-api.ts'; import { ChatMessageActions, MessageEvents } from '../../src/core/events.ts'; import { Message } from '../../src/core/message.ts'; -import { DefaultMessages, MessageEventPayload } from '../../src/core/messages.ts'; +import { DefaultMessages, MessageEventPayload, ResultOrder } from '../../src/core/messages.ts'; import { Room } from '../../src/core/room.ts'; import { channelEventEmitter, @@ -638,14 +638,14 @@ describe('Messages', () => { it('should query listener history with the attachment serial after attaching', async (context) => { const testAttachSerial = '01672531200000-123@abcdefghij'; - const testDirection = 'backwards'; + const testOrderBy = ResultOrder.NewestFirst; const testLimit = 50; const { room, chatApi } = context; vi.spyOn(chatApi, 'getMessages').mockImplementation((roomId, params): Promise> => { expect(roomId).toEqual(room.roomId); - expect(params.direction).toEqual(testDirection); + expect(params.orderBy).toEqual(testOrderBy); expect(params.limit).toEqual(testLimit); expect(params.fromSerial).toEqual(testAttachSerial); return Promise.resolve(mockPaginatedResultWithItems([])); @@ -698,14 +698,14 @@ describe('Messages', () => { it('should query listener history with latest channel serial if already attached to the channel', async (context) => { // We should use the latest channel serial if we are already attached to the channel const latestChannelSerial = '01672531200000-123@abcdefghij'; - const testDirection = 'backwards'; + const testOrderBy = ResultOrder.NewestFirst; const testLimit = 50; const { room, chatApi } = context; vi.spyOn(chatApi, 'getMessages').mockImplementation((roomId, params): Promise> => { expect(roomId).toEqual(room.roomId); - expect(params.direction).toEqual(testDirection); + expect(params.orderBy).toEqual(testOrderBy); expect(params.limit).toEqual(testLimit); expect(params.fromSerial).toEqual(latestChannelSerial); return Promise.resolve(mockPaginatedResultWithItems([])); @@ -736,7 +736,7 @@ describe('Messages', () => { it('when attach occurs, should query with correct params if listener registered before attach', async (context) => { const firstAttachmentSerial = '01772531200000-001@108uyDJAgBOihn12345678'; - const testDirection = 'backwards'; + const testOrderBy = ResultOrder.NewestFirst; const testLimit = 50; let expectFunction: (roomId: string, params: GetMessagesQueryParams) => void = () => {}; @@ -781,7 +781,7 @@ describe('Messages', () => { // Check we are using the attachSerial expectFunction = (roomId: string, params: GetMessagesQueryParams) => { expect(roomId).toEqual(room.roomId); - expect(params.direction).toEqual(testDirection); + expect(params.orderBy).toEqual(testOrderBy); expect(params.limit).toEqual(testLimit); expect(params.fromSerial).toEqual(firstAttachmentSerial); }; @@ -833,7 +833,7 @@ describe('Messages', () => { // Testing the case where the channel is already attached and we have a channel serial set const firstChannelSerial = '01992531200000-001@abghhDJ2dBOihn12345678'; const firstAttachSerial = '01992531200000-001@ackhhDJ2dBOihn12345678'; - const testDirection = 'backwards'; + const testOrderBy = ResultOrder.NewestFirst; const testLimit = 50; let expectFunction: (roomId: string, params: GetMessagesQueryParams) => void = () => {}; @@ -870,7 +870,7 @@ describe('Messages', () => { // Check we are using the channel serial expectFunction = (roomId: string, params: GetMessagesQueryParams) => { expect(roomId).toEqual(room.roomId); - expect(params.direction).toEqual(testDirection); + expect(params.orderBy).toEqual(testOrderBy); expect(params.limit).toEqual(testLimit); expect(params.fromSerial).toEqual(firstChannelSerial); }; @@ -921,7 +921,7 @@ describe('Messages', () => { const firstChannelSerial = '01992531200000-001@108hhDJ2hpInKn12345678'; const firstAttachSerial = '01992531200000-001@108hhDJBiKOihn12345678'; - const testDirection = 'backwards'; + const testOrderBy = ResultOrder.NewestFirst; const testLimit = 50; let expectFunction: (roomId: string, params: GetMessagesQueryParams) => void = () => {}; @@ -959,7 +959,7 @@ describe('Messages', () => { // Check we are using the channel serial expectFunction = (roomId: string, params: GetMessagesQueryParams) => { expect(roomId).toEqual(room.roomId); - expect(params.direction).toEqual(testDirection); + expect(params.orderBy).toEqual(testOrderBy); expect(params.limit).toEqual(testLimit); expect(params.fromSerial).toEqual(firstChannelSerial); };