Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/alpha' into fix/error-responses-…
Browse files Browse the repository at this point in the history
…to-deno
  • Loading branch information
d-gubert committed Jul 29, 2024
2 parents 994608a + 4bb6dec commit d097163
Show file tree
Hide file tree
Showing 12 changed files with 230 additions and 77 deletions.
61 changes: 33 additions & 28 deletions deno-runtime/handlers/tests/videoconference-handler.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// deno-lint-ignore-file no-explicit-any
import { assertEquals, assertObjectMatch } from 'https://deno.land/[email protected]/assert/mod.ts';
import { beforeEach, describe, it } from 'https://deno.land/[email protected]/testing/bdd.ts';
import { spy } from "https://deno.land/[email protected]/testing/mock.ts";
import { spy } from 'https://deno.land/[email protected]/testing/mock.ts';

import { AppObjectRegistry } from '../../AppObjectRegistry.ts';
import videoconfHandler from '../videoconference-handler.ts';
import { assertInstanceOf } from "https://deno.land/[email protected]/assert/assert_instance_of.ts";
import { JsonRpcError } from "jsonrpc-lite";
import { assertInstanceOf } from 'https://deno.land/[email protected]/assert/assert_instance_of.ts';
import { JsonRpcError } from 'jsonrpc-lite';

describe('handlers > videoconference', () => {
// deno-lint-ignore no-unused-vars
Expand All @@ -16,15 +16,18 @@ describe('handlers > videoconference', () => {
// deno-lint-ignore no-unused-vars
const mockMethodWithTwoParam = (call: any, user: any, read: any, modify: any, http: any, persis: any): Promise<string> => Promise.resolve('ok two');
// deno-lint-ignore no-unused-vars
const mockMethodWithThreeParam = (call: any, user: any, options: any, read: any, modify: any, http: any, persis: any): Promise<string> => Promise.resolve('ok three');
const mockMethodWithThreeParam = (call: any, user: any, options: any, read: any, modify: any, http: any, persis: any): Promise<string> =>
Promise.resolve('ok three');
const mockProvider = {
empty: mockMethodWithoutParam,
one: mockMethodWithOneParam,
two: mockMethodWithTwoParam,
three: mockMethodWithThreeParam,
notAFunction: true,
error: () => { throw new Error('Method execution error example') }
}
error: () => {
throw new Error('Method execution error example');
},
};

beforeEach(() => {
AppObjectRegistry.clear();
Expand All @@ -36,9 +39,9 @@ describe('handlers > videoconference', () => {

const result = await videoconfHandler('videoconference:test-provider:empty', []);

assertEquals(result, 'ok none')
assertEquals(result, 'ok none');
assertEquals(_spy.calls[0].args.length, 4);

_spy.restore();
});

Expand All @@ -47,7 +50,7 @@ describe('handlers > videoconference', () => {

const result = await videoconfHandler('videoconference:test-provider:one', ['call']);

assertEquals(result, 'ok one')
assertEquals(result, 'ok one');
assertEquals(_spy.calls[0].args.length, 5);
assertEquals(_spy.calls[0].args[0], 'call');

Expand All @@ -59,7 +62,7 @@ describe('handlers > videoconference', () => {

const result = await videoconfHandler('videoconference:test-provider:two', ['call', 'user']);

assertEquals(result, 'ok two')
assertEquals(result, 'ok two');
assertEquals(_spy.calls[0].args.length, 6);
assertEquals(_spy.calls[0].args[0], 'call');
assertEquals(_spy.calls[0].args[1], 'user');
Expand All @@ -72,7 +75,7 @@ describe('handlers > videoconference', () => {

const result = await videoconfHandler('videoconference:test-provider:three', ['call', 'user', 'options']);

assertEquals(result, 'ok three')
assertEquals(result, 'ok three');
assertEquals(_spy.calls[0].args.length, 7);
assertEquals(_spy.calls[0].args[0], 'call');
assertEquals(_spy.calls[0].args[1], 'user');
Expand All @@ -84,34 +87,36 @@ describe('handlers > videoconference', () => {
it('correctly handles an error on execution of a videoconf method', async () => {
const result = await videoconfHandler('videoconference:test-provider:error', []);

assertInstanceOf(result, JsonRpcError)
assertInstanceOf(result, JsonRpcError);
assertObjectMatch(result, {
message: 'Method execution error example',
code: -32000
})
})
code: -32000,
});
});

it('correctly handles an error when provider is not found', async () => {
const providerName = 'error-provider'
const providerName = 'error-provider';
const result = await videoconfHandler(`videoconference:${providerName}:method`, []);

assertInstanceOf(result, JsonRpcError)
assertInstanceOf(result, JsonRpcError);
assertObjectMatch(result, {
message: `Provider ${providerName} not found`,
code: -32000
})
})
code: -32000,
});
});

it('correctly handles an error if method is not a function of provider', async () => {
const methodName = 'notAFunction'
const providerName = 'test-provider'
const methodName = 'notAFunction';
const providerName = 'test-provider';
const result = await videoconfHandler(`videoconference:${providerName}:${methodName}`, []);

assertInstanceOf(result, JsonRpcError)
assertInstanceOf(result, JsonRpcError);
assertObjectMatch(result, {
message: `Method ${methodName} not found on provider ${providerName}`,
code: -32000
})
})

message: 'Method not found',
code: -32601,
data: {
message: `Method ${methodName} not found on provider ${providerName}`,
},
});
});
});
4 changes: 3 additions & 1 deletion deno-runtime/handlers/videoconference-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export default async function videoConferenceHandler(call: string, params: unkno
const method = provider[methodName as keyof IVideoConfProvider];

if (typeof method !== 'function') {
return new JsonRpcError(`Method ${methodName} not found on provider ${providerName}`, -32000);
return JsonRpcError.methodNotFound({
message: `Method ${methodName} not found on provider ${providerName}`,
});
}

const [videoconf, user, options] = params as Array<unknown>;
Expand Down
15 changes: 10 additions & 5 deletions src/definition/accessors/IRoomRead.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { IMessage } from '../messages/index';
import type { GetMessagesOptions } from '../../server/bridges/RoomBridge';
import type { IMessageRaw } from '../messages/index';
import type { IRoom } from '../rooms/index';
import type { IUser } from '../users/index';

Expand Down Expand Up @@ -40,12 +41,16 @@ export interface IRoomRead {
getCreatorUserByName(name: string): Promise<IUser | undefined>;

/**
* Gets an iterator for all of the messages in the provided room.
* Retrieves an array of messages from the specified room.
*
* @param roomId the room's id
* @returns an iterator for messages
* @param roomId The unique identifier of the room from which to retrieve messages.
* @param options Optional parameters for retrieving messages:
* - limit: The maximum number of messages to retrieve. Maximum 100
* - skip: The number of messages to skip (for pagination).
* - sort: An object defining the sorting order of the messages. Each key is a field to sort by, and the value is either "asc" for ascending order or "desc" for descending order.
* @returns A Promise that resolves to an array of IMessage objects representing the messages in the room.
*/
getMessages(roomId: string): Promise<IterableIterator<IMessage>>;
getMessages(roomId: string, options?: Partial<GetMessagesOptions>): Promise<Array<IMessageRaw>>;

/**
* Gets an iterator for all of the users in the provided room.
Expand Down
40 changes: 40 additions & 0 deletions src/definition/messages/IMessageRaw.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import type { IBlock, Block } from '@rocket.chat/ui-kit';

import type { IRoom } from '../rooms';
import type { IUserLookup } from '../users';
import type { IMessageAttachment } from './IMessageAttachment';
import type { IMessageFile } from './IMessageFile';
import type { IMessageReactions } from './IMessageReaction';

/**
* The raw version of a message, without resolved information for relationship fields, i.e.
* `room`, `sender` and `editor` are not the complete entity like they are in `IMessage`
*
* This is used in methods that fetch multiple messages at the same time, as resolving the relationship
* fields require additional queries to the database and would hit the system's performance significantly.
*/
export interface IMessageRaw {
id: string;
roomId: IRoom['id'];
sender: IUserLookup;
createdAt: Date;
threadId?: string;
text?: string;
updatedAt?: Date;
editor?: IUserLookup;
editedAt?: Date;
emoji?: string;
avatarUrl?: string;
alias?: string;
file?: IMessageFile;
attachments?: Array<IMessageAttachment>;
reactions?: IMessageReactions;
groupable?: boolean;
parseUrls?: boolean;
customFields?: { [key: string]: any };
blocks?: Array<IBlock | Block>;
starred?: Array<{ _id: string }>;
pinned?: boolean;
pinnedAt?: Date;
pinnedBy?: IUserLookup;
}
2 changes: 2 additions & 0 deletions src/definition/messages/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { IMessageDeleteContext } from './IMessageDeleteContext';
import { IMessageFile } from './IMessageFile';
import { IMessageFollowContext } from './IMessageFollowContext';
import { IMessagePinContext } from './IMessagePinContext';
import { IMessageRaw } from './IMessageRaw';
import { IMessageReaction, IMessageReactions } from './IMessageReaction';
import { IMessageReactionContext } from './IMessageReactionContext';
import { IMessageReportContext } from './IMessageReportContext';
Expand Down Expand Up @@ -39,6 +40,7 @@ export {
IMessageAttachmentField,
IMessageAction,
IMessageFile,
IMessageRaw,
IMessageReactions,
IMessageReaction,
IPostMessageDeleted,
Expand Down
1 change: 1 addition & 0 deletions src/definition/users/IUserLookup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export interface IUserLookup {
_id: string;
username: string;
name?: string;
}
30 changes: 27 additions & 3 deletions src/server/accessors/RoomRead.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { IRoomRead } from '../../definition/accessors';
import type { IMessage } from '../../definition/messages';
import type { IMessageRaw } from '../../definition/messages';
import type { IRoom } from '../../definition/rooms';
import type { IUser } from '../../definition/users';
import type { RoomBridge } from '../bridges';
import { type GetMessagesOptions, GetMessagesSortableFields } from '../bridges/RoomBridge';

export class RoomRead implements IRoomRead {
constructor(private roomBridge: RoomBridge, private appId: string) {}
Expand All @@ -23,8 +24,18 @@ export class RoomRead implements IRoomRead {
return this.roomBridge.doGetCreatorByName(name, this.appId);
}

public getMessages(roomId: string): Promise<IterableIterator<IMessage>> {
throw new Error('Method not implemented.');
public getMessages(roomId: string, options: Partial<GetMessagesOptions> = {}): Promise<IMessageRaw[]> {
if (typeof options.limit !== 'undefined' && (!Number.isFinite(options.limit) || options.limit > 100)) {
throw new Error(`Invalid limit provided. Expected number <= 100, got ${options.limit}`);
}

options.limit ??= 100;

if (options.sort) {
this.validateSort(options.sort);
}

return this.roomBridge.doGetMessages(roomId, options as GetMessagesOptions, this.appId);
}

public getMembers(roomId: string): Promise<Array<IUser>> {
Expand All @@ -46,4 +57,17 @@ export class RoomRead implements IRoomRead {
public getLeaders(roomId: string): Promise<Array<IUser>> {
return this.roomBridge.doGetLeaders(roomId, this.appId);
}

// If there are any invalid fields or values, throw
private validateSort(sort: Record<string, unknown>) {
Object.entries(sort).forEach(([key, value]) => {
if (!GetMessagesSortableFields.includes(key as typeof GetMessagesSortableFields[number])) {
throw new Error(`Invalid key "${key}" used in sort. Available keys for sorting are ${GetMessagesSortableFields.join(', ')}`);
}

if (value !== 'asc' && value !== 'desc') {
throw new Error(`Invalid sort direction for field "${key}". Expected "asc" or "desc", got ${value}`);
}
});
}
}
18 changes: 17 additions & 1 deletion src/server/bridges/RoomBridge.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
import type { IMessage } from '../../definition/messages';
import type { IMessage, IMessageRaw } from '../../definition/messages';
import type { IRoom } from '../../definition/rooms';
import type { IUser } from '../../definition/users';
import { PermissionDeniedError } from '../errors/PermissionDeniedError';
import { AppPermissionManager } from '../managers/AppPermissionManager';
import { AppPermissions } from '../permissions/AppPermissions';
import { BaseBridge } from './BaseBridge';

export const GetMessagesSortableFields = ['createdAt'] as const;

export type GetMessagesOptions = {
limit: number;
skip: number;
sort: Record<typeof GetMessagesSortableFields[number], 'asc' | 'desc'>;
};

export abstract class RoomBridge extends BaseBridge {
public async doCreate(room: IRoom, members: Array<string>, appId: string): Promise<string> {
if (this.hasWritePermission(appId)) {
Expand Down Expand Up @@ -91,6 +99,12 @@ export abstract class RoomBridge extends BaseBridge {
}
}

public async doGetMessages(roomId: string, options: GetMessagesOptions, appId: string): Promise<IMessageRaw[]> {
if (this.hasReadPermission(appId)) {
return this.getMessages(roomId, options, appId);
}
}

public async doRemoveUsers(roomId: string, usernames: Array<string>, appId: string): Promise<void> {
if (this.hasWritePermission(appId)) {
return this.removeUsers(roomId, usernames, appId);
Expand Down Expand Up @@ -129,6 +143,8 @@ export abstract class RoomBridge extends BaseBridge {

protected abstract getLeaders(roomId: string, appId: string): Promise<Array<IUser>>;

protected abstract getMessages(roomId: string, options: GetMessagesOptions, appId: string): Promise<IMessageRaw[]>;

protected abstract removeUsers(roomId: string, usernames: Array<string>, appId: string): Promise<void>;

private hasWritePermission(appId: string): boolean {
Expand Down
12 changes: 11 additions & 1 deletion src/server/managers/AppVideoConfProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { VideoConference } from '../../definition/videoConferences';
import type { IVideoConferenceUser } from '../../definition/videoConferences/IVideoConferenceUser';
import type { IVideoConferenceOptions, IVideoConfProvider, VideoConfData, VideoConfDataExtended } from '../../definition/videoConfProviders';
import type { ProxiedApp } from '../ProxiedApp';
import { JSONRPC_METHOD_NOT_FOUND } from '../runtime/deno/AppsEngineDenoRuntime';
import type { AppLogStorage } from '../storage';
import type { AppAccessorManager } from './AppAccessorManager';

Expand Down Expand Up @@ -86,8 +87,17 @@ export class AppVideoConfProvider {
params: runContextArgs,
});

return result as string;
return result as string | boolean | Array<IBlock> | undefined;
} catch (e) {
if (e?.code === JSONRPC_METHOD_NOT_FOUND) {
if (method === AppMethod._VIDEOCONF_IS_CONFIGURED) {
return true;
}
if (![AppMethod._VIDEOCONF_GENERATE_URL, AppMethod._VIDEOCONF_CUSTOMIZE_URL].includes(method)) {
return undefined;
}
}

// @TODO add error handling
console.log(e);
}
Expand Down
Loading

0 comments on commit d097163

Please sign in to comment.