Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: optional methods in video conf providers not being handled correctly #785

Merged
merged 3 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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