Skip to content

Commit

Permalink
core: change direction to orderBy for history, use enum
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
AndyTWF committed Nov 29, 2024
1 parent 58a4fa2 commit 34ff9c6
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 34 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 22 additions & 4 deletions src/core/chat-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -96,9 +105,18 @@ 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) {
apiParams.direction = params.orderBy === ResultOrder.OldestFirst ? 'forwards' : 'backwards';
}

return this._makeAuthorizedPaginatedRequest<Message>(`/chat/v2/rooms/${roomId}/messages`, apiParams).then(
(data) => {
return this._recursivePaginateMessages(data);
},
);
}

private _recursivePaginateMessages(data: PaginatedResult<Message>): PaginatedResult<Message> {
Expand Down
1 change: 1 addition & 0 deletions src/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export type {
Messages,
MessageSubscriptionResponse,
QueryOptions,
ResultOrder,
SendMessageParams,
UpdateMessageParams,
} from './messages.js';
Expand Down
33 changes: 24 additions & 9 deletions src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.
*/
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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>>;
}

/**
Expand Down Expand Up @@ -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();`);

Expand All @@ -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,
});
}
Expand Down Expand Up @@ -584,7 +599,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),
};
}
Expand Down
4 changes: 3 additions & 1 deletion src/react/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,9 @@ 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: ResultOrder.oldestFirst }).then((result) =>
console.log('Previous messages: ', result.items),
);
};

const handleMessageSend = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/react/hooks/use-messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
15 changes: 8 additions & 7 deletions test/core/messages.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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([

Check failure on line 224 in test/core/messages.integration.test.ts

View workflow job for this annotation

GitHub Actions / test

test/core/messages.integration.test.ts > messages integration > should be able to retrieve chat history

AssertionError: expected [] to deeply equal [ ObjectContaining{…}, …(2) ] - Expected + Received - Array [ - ObjectContaining { - "clientId": "ably-chat-js-client-h1p5e", - "serial": "01733140054079-000@e7d-AgH_gBjCoT27117263:000", - "text": "Hello there!", - }, - ObjectContaining { - "clientId": "ably-chat-js-client-h1p5e", - "serial": "01733140054101-000@e7d-AgH_gBjCoT27117263:000", - "text": "I have the high ground!", - }, - ObjectContaining { - "clientId": "ably-chat-js-client-h1p5e", - "serial": "01733140054119-000@e7d-AgH_gBjCoT27117263:000", - "text": "You underestimate my power!", - }, - ] + Array [] ❯ test/core/messages.integration.test.ts:224:27
expect.objectContaining({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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([

Check failure on line 479 in test/core/messages.integration.test.ts

View workflow job for this annotation

GitHub Actions / test

test/core/messages.integration.test.ts > messages integration > should be able to paginate chat history, but backwards

AssertionError: expected [] to deeply equal [ ObjectContaining{…}, …(2) ] - Expected + Received - Array [ - ObjectContaining { - "clientId": "ably-chat-js-client-g33xaw", - "serial": "01733140056164-000@e7dzKNduABjCoS68422340:000", - "text": "Don't try it!", - }, - ObjectContaining { - "clientId": "ably-chat-js-client-g33xaw", - "serial": "01733140056147-000@e7dzKNduABjCoS68422340:000", - "text": "You underestimate my power!", - }, - ObjectContaining { - "clientId": "ably-chat-js-client-g33xaw", - "serial": "01733140056131-000@e7dzKNduABjCoS68422340:000", - "text": "I have the high ground!", - }, - ] + Array [] ❯ test/core/messages.integration.test.ts:479:28
expect.objectContaining({
Expand Down Expand Up @@ -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([
Expand Down
22 changes: 11 additions & 11 deletions test/core/messages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -638,14 +638,14 @@ describe('Messages', () => {

it<TestContext>('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<Ably.PaginatedResult<Message>> => {
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([]));
Expand Down Expand Up @@ -698,14 +698,14 @@ describe('Messages', () => {
it<TestContext>('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<Ably.PaginatedResult<Message>> => {
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([]));
Expand Down Expand Up @@ -736,7 +736,7 @@ describe('Messages', () => {

it<TestContext>('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 = () => {};
Expand Down Expand Up @@ -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);
};
Expand Down Expand Up @@ -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 = () => {};
Expand Down Expand Up @@ -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);
};
Expand Down Expand Up @@ -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 = () => {};
Expand Down Expand Up @@ -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);
};
Expand Down

0 comments on commit 34ff9c6

Please sign in to comment.