Merge pull request #420 from ably/chat-history-direction

core: change direction to orderBy for history, use enum

README.md

@@ -287,17 +287,20 @@ To update an existing message, call `update` on the `room.messages` property, wi
 the updated fields, and optional operation details to provide extra context for the update.
 
 The optional operation details are:
-* `description`: a string that can be used to inform others as to why the message was updated.
-* `metadata`: a map of extra information that can be attached to the update operation.
+
+- `description`: a string that can be used to inform others as to why the message was updated.
+- `metadata`: a map of extra information that can be attached to the update operation.
 
 Example
+
 ```typescript
-const updatedMessage = await room.messages.update(message,
+const updatedMessage = await room.messages.update(
+  message,
   {
-    text: "hello, this is edited",
+    text: 'hello, this is edited',
   },
   {
-    description: "edit example",
+    description: 'edit example',
   },
 );
 ```
@@ -317,10 +320,11 @@ In rare occasions updates might arrive over realtime out of order. To keep a cor
 The same out-of-order situation can happen between updates received over realtime and HTTP responses. In the situation where two concurrent updates happen, both might be received via realtime before the HTTP response of the first one arrives. Always compare the message `version` to determine which instance of a `Message` is newer.
 
 Example for handling updates:
+
 ```typescript
-const messages : Message[] = []; // assuming this is where state is kept
+const messages: Message[] = []; // assuming this is where state is kept
 
-room.messages.subscribe(event => {
+room.messages.subscribe((event) => {
   switch (event.type) {
     case MessageEvents.Updated: {
       const serial = event.message.serial;
@@ -342,18 +346,19 @@ To delete a message, call `delete` on the `room.messages` property, with the ori
 You can supply optional parameters to the `delete` method to provide additional context for the deletion.
 
 These additional parameters are:
-* `description`: a string that can be used to inform others as to why the message was deleted.
-* `metadata`: a map of extra information that can be attached to the deletion message.
+
+- `description`: a string that can be used to inform others as to why the message was deleted.
+- `metadata`: a map of extra information that can be attached to the deletion message.
 
 The return of this call will be the deleted message, as it would appear to other subscribers of the room.
 This is a _soft delete_ and the message will still be available in the history.
 
 Example
+
 ```ts
-const deletedMessage = await room.messages.delete(message,
-        { 
-          description: 'This message was deleted for ...'
-        });
+const deletedMessage = await room.messages.delete(message, {
+  description: 'This message was deleted for ...',
+});
 ```
 
 `deletedMessage` is a Message object with the deletion applied. As with sending, the promise may resolve after the deletion message is received via the messages subscription.
@@ -386,10 +391,11 @@ In short, always use `actionAfter()`,
 `actionBefore()`, or `actionEqual()` to determine the global ordering of two `Message` actions.
 
 Example for handling deletes:
+
 ```typescript
-const messages : Message[] = []; // assuming this is where state is kept
+const messages: Message[] = []; // assuming this is where state is kept
 
-room.messages.subscribe(event => {
+room.messages.subscribe((event) => {
   switch (event.type) {
     case MessageEvents.Deleted: {
       const serial = event.message.serial;
@@ -401,8 +407,9 @@ room.messages.subscribe(event => {
     }
     // other event types (ie. created and updated) omitted
   }
-})
+});
 ```
+
 ### Subscribing to incoming messages
 
 To subscribe to incoming messages, call `subscribe` with your listener.
@@ -431,7 +438,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: OrderBy.NewestFirst, limit: 50 });
 console.log(historicalMessages.items);
 if (historicalMessages.hasNext()) {
   const next = await historicalMessages.next();

src/core/chat-api.ts

@@ -2,13 +2,14 @@ import * as Ably from 'ably';
 
 import { Logger } from './logger.js';
 import { DefaultMessage, Message, MessageHeaders, MessageMetadata, MessageOperationMetadata } from './message.js';
+import { OrderBy } from './messages.js';
 import { OccupancyEvent } from './occupancy.js';
 import { PaginatedResult } from './query.js';
 
 export interface GetMessagesQueryParams {
   start?: number;
   end?: number;
-  direction?: 'forwards' | 'backwards';
+  orderBy?: OrderBy;
   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<GetMessagesQueryParams, 'orderBy'> & {
+  direction?: 'forwards' | 'backwards';
+};
+
 export interface CreateMessageResponse {
   serial: string;
   createdAt: number;
@@ -91,9 +100,29 @@ export class ChatApi {
 
   async getMessages(roomId: string, params: GetMessagesQueryParams): Promise<PaginatedResult<Message>> {
     roomId = encodeURIComponent(roomId);
-    return this._makeAuthorizedPaginatedRequest<Message>(`/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) {
+      switch (params.orderBy) {
+        case OrderBy.NewestFirst: {
+          apiParams.direction = 'backwards';
+          break;
+        }
+        case OrderBy.OldestFirst: {
+          apiParams.direction = 'forwards';
+          break;
+        }
+        default: {
+          // in vanilla JS use-cases, without types, we need to check non-enum values
+          // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+          throw new Ably.ErrorInfo(`invalid orderBy value: ${params.orderBy}`, 40000, 400);
+        }
+      }
+    }
+
+    const data = await this._makeAuthorizedPaginatedRequest<Message>(`/chat/v2/rooms/${roomId}/messages`, apiParams);
+    return this._recursivePaginateMessages(data);
   }
 
   private _recursivePaginateMessages(data: PaginatedResult<Message>): PaginatedResult<Message> {

src/core/index.ts

@@ -34,6 +34,7 @@ export type {
   Messages,
   MessageSubscriptionResponse,
   OperationDetails,
+  OrderBy,
   QueryOptions,
   SendMessageParams,
   UpdateMessageParams,

src/core/messages.ts

@@ -39,6 +39,21 @@ const MessageActionsToEventsMap: Map<ChatMessageActions, MessageEvents> = 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 OrderBy {
+  /**
+   * 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 `backwards`.
+   * If {@link OrderBy.OldestFirst}, the response will include messages from the start of the time window to the end.
+   * If {@link OrderBy.NewestFirst}, the response will include messages from the end of the time window to the start.
+   * If not provided, the default is {@link OrderBy.NewestFirst}.
    *
-   * @defaultValue backwards
+   * @defaultValue {@link OrderBy.NewestFirst}
    */
-  direction?: 'forwards' | 'backwards';
+  orderBy?: OrderBy;
 }
 
 /**
@@ -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<QueryOptions, 'direction'>): Promise<PaginatedResult<Message>>;
+  getPreviousMessages(params: Omit<QueryOptions, 'orderBy'>): Promise<PaginatedResult<Message>>;
 }
 
 /**
@@ -351,7 +366,7 @@ export class DefaultMessages
    */
   private async _getBeforeSubscriptionStart(
     listener: MessageListener,
-    params: Omit<QueryOptions, 'direction'>,
+    params: Omit<QueryOptions, 'orderBy'>,
   ): Promise<PaginatedResult<Message>> {
     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: OrderBy.NewestFirst,
       ...subscriptionPointParams,
     });
   }
@@ -589,7 +604,7 @@ export class DefaultMessages
         this._logger.trace('Messages.unsubscribe();');
         super.off(listener);
       },
-      getPreviousMessages: (params: Omit<QueryOptions, 'direction'>) =>
+      getPreviousMessages: (params: Omit<QueryOptions, 'orderBy'>) =>
         this._getBeforeSubscriptionStart(listener, params),
     };
   }

src/react/README.md

@@ -225,7 +225,7 @@ const MyComponent = () => {
   const [message, setMessage] = useState<Message>();
   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: OrderBy.oldestFirst }).then((result) => console.log('Previous messages: ', result.items));
   };
 
   const handleMessageSend = () => {

src/react/hooks/use-messages.ts

@@ -145,7 +145,7 @@ export const useMessages = (params?: UseMessagesParams): UseMessagesResponse =>
             return;
           }
 
-          return (params: Omit<QueryOptions, 'direction'>) => {
+          return (params: Omit<QueryOptions, 'orderBy'>) => {
             // 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 });

test/core/chat-api.test.ts

@@ -70,4 +70,20 @@ describe('config', () => {
       statusCode: 400,
     });
   });
+
+  it('throws errors if invalid OrderBy used on history request', async () => {
+    const realtime = new Ably.Realtime({ clientId: 'test' });
+    const chatApi = new ChatApi(realtime, makeTestLogger());
+
+    vi.spyOn(realtime, 'request');
+
+    // @ts-expect-error Testing invalid OrderBy
+    await expect(chatApi.getMessages('test', { orderBy: 'foo' })).rejects.toBeErrorInfo({
+      message: 'invalid orderBy value: foo',
+      code: 40000,
+      statusCode: 400,
+    });
+
+    expect(realtime.request).not.toHaveBeenCalled();
+  });
 });

test/core/messages.integration.test.ts

@@ -1,9 +1,10 @@
-import { ChatMessageActions, Message } from '@ably/chat';
 import * as Ably from 'ably';
 import { beforeEach, describe, expect, it } from 'vitest';
 
 import { ChatClient } from '../../src/core/chat.ts';
-import { MessageEvents } from '../../src/core/events.ts';
+import { ChatMessageActions, MessageEvents } from '../../src/core/events.ts';
+import { Message } from '../../src/core/message.ts';
+import { OrderBy } 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';
@@ -230,7 +231,7 @@ describe('messages integration', { timeout: 10000 }, () => {
 
     // Do a history request to get all 3 messages
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history = await room.messages.get({ limit: 3, direction: 'forwards' });
+    const history = await room.messages.get({ limit: 3, orderBy: OrderBy.OldestFirst });
 
     expect(history.items).toEqual([
       expect.objectContaining({
@@ -267,7 +268,7 @@ describe('messages integration', { timeout: 10000 }, () => {
 
     // Do a history request to get the deleted message
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history = await room.messages.get({ limit: 3, direction: 'forwards' });
+    const history = await room.messages.get({ limit: 3, orderBy: OrderBy.OldestFirst });
 
     expect(history.items).toEqual([
       expect.objectContaining({
@@ -308,7 +309,7 @@ describe('messages integration', { timeout: 10000 }, () => {
 
     // Do a history request to get the update message
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history = await room.messages.get({ limit: 3, direction: 'forwards' });
+    const history = await room.messages.get({ limit: 3, orderBy: OrderBy.OldestFirst });
 
     expect(history.items).toEqual([
       expect.objectContaining({
@@ -345,7 +346,7 @@ describe('messages integration', { timeout: 10000 }, () => {
 
     // Do a history request to get the first 3 messages
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history1 = await room.messages.get({ limit: 3, direction: 'forwards' });
+    const history1 = await room.messages.get({ limit: 3, orderBy: OrderBy.OldestFirst });
 
     expect(history1.items).toEqual([
       expect.objectContaining({
@@ -451,7 +452,7 @@ describe('messages integration', { timeout: 10000 }, () => {
 
     // Do a history request to get the first 3 messages
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history1 = await room.messages.get({ limit: 3, direction: 'forwards' });
+    const history1 = await room.messages.get({ limit: 3, orderBy: OrderBy.OldestFirst });
 
     expect(history1.items).toEqual([
       expect.objectContaining({
@@ -502,7 +503,7 @@ describe('messages integration', { timeout: 10000 }, () => {
 
     // Do a history request to get the last 3 messages
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history1 = await room.messages.get({ limit: 3, direction: 'backwards' });
+    const history1 = await room.messages.get({ limit: 3, orderBy: OrderBy.NewestFirst });
 
     expect(history1.items).toEqual([
       expect.objectContaining({
@@ -591,7 +592,7 @@ describe('messages integration', { timeout: 10000 }, () => {
     ]);
 
     await new Promise((resolve) => setTimeout(resolve, 3000)); // wait for persistence - this will not be necessary in the future
-    const history = await room.messages.get({ limit: 2, direction: 'forwards' });
+    const history = await room.messages.get({ limit: 2, orderBy: OrderBy.OldestFirst });
 
     expect(history.items.length).toEqual(2);
     expect(history.items, 'history messages to match').toEqual([