From 71401493738e28ea90203a667b307816a5e4feed Mon Sep 17 00:00:00 2001 From: Junyao Chen Date: Fri, 21 Jul 2023 04:41:35 -0400 Subject: [PATCH] ts: Migrate `bot_data.js` to TypeScript. Add type annotations. Create custom types for Bot and Service. Add zod data validation for incoming bot data from server. Based on `zerver/openapi/zulip.yaml` description, `add` operation (`op`) carries data that follows `Bot` structure. So taking reference from `bot` structure, I create `ServerAddBotData` zod schema and infer its type. Similarly, `update` operation carries data that follows `BasicBot`, so I create `ServerUpdateBotData`. Note that `Bot` inherits from `BasicBot`. `zerver/openapi/zulip.yaml` describes that `services` in `BasicBot` can be one of two objects, one with `{base_url, token, interface}`, another with `{service_name, config_data}`. Therefore, I create two corresponding schema and infer their types. Fix two test cases `bot_data.test.js` and `settings_bots.test.js` whose synthetic objects should have had followed the schema. --- web/src/bot_data.js | 99 ----------------------- web/src/bot_data.ts | 134 ++++++++++++++++++++++++++++++++ web/tests/bot_data.test.js | 102 ++++++++++++++++++++---- web/tests/settings_bots.test.js | 9 +++ 4 files changed, 230 insertions(+), 114 deletions(-) delete mode 100644 web/src/bot_data.js create mode 100644 web/src/bot_data.ts diff --git a/web/src/bot_data.js b/web/src/bot_data.js deleted file mode 100644 index 3d4e53c839..0000000000 --- a/web/src/bot_data.js +++ /dev/null @@ -1,99 +0,0 @@ -import _ from "lodash"; - -import * as people from "./people"; - -const bots = new Map(); - -const bot_fields = [ - "api_key", - "avatar_url", - "bot_type", - "default_all_public_streams", - "default_events_register_stream", - "default_sending_stream", - "email", - "full_name", - "is_active", - "owner", // TODO: eliminate - "owner_id", - "user_id", -]; - -const services = new Map(); -const services_fields = ["base_url", "interface", "config_data", "service_name", "token"]; - -export function all_user_ids() { - return [...bots.keys()]; -} - -export function add(bot) { - const clean_bot = _.pick(bot, bot_fields); - bots.set(bot.user_id, clean_bot); - const clean_services = bot.services.map((service) => _.pick(service, services_fields)); - services.set(bot.user_id, clean_services); -} - -export function deactivate(bot_id) { - bots.get(bot_id).is_active = false; -} - -export function del(bot_id) { - bots.delete(bot_id); - services.delete(bot_id); -} - -export function update(bot_id, bot_update) { - const bot = bots.get(bot_id); - Object.assign(bot, _.pick(bot_update, bot_fields)); - - // We currently only support one service per bot. - const service = services.get(bot_id)[0]; - if (bot_update.services !== undefined && bot_update.services.length > 0) { - Object.assign(service, _.pick(bot_update.services[0], services_fields)); - } -} - -export function get_all_bots_for_current_user() { - const ret = []; - for (const bot of bots.values()) { - if (people.is_my_user_id(bot.owner_id)) { - ret.push(bot); - } - } - return ret; -} - -export function get_editable() { - const ret = []; - for (const bot of bots.values()) { - if (bot.is_active && people.is_my_user_id(bot.owner_id)) { - ret.push(bot); - } - } - return ret; -} - -export function get_all_bots_owned_by_user(user_id) { - const ret = []; - for (const bot of bots.values()) { - if (bot.owner_id === user_id && bot.is_active) { - ret.push(bot); - } - } - return ret; -} - -export function get(bot_id) { - return bots.get(bot_id); -} - -export function get_services(bot_id) { - return services.get(bot_id); -} - -export function initialize(params) { - bots.clear(); - for (const bot of params.realm_bots) { - add(bot); - } -} diff --git a/web/src/bot_data.ts b/web/src/bot_data.ts new file mode 100644 index 0000000000..f9a3b64591 --- /dev/null +++ b/web/src/bot_data.ts @@ -0,0 +1,134 @@ +import {z} from "zod"; + +import * as people from "./people"; + +export type ServerUpdateBotData = z.infer; +export type ServerAddBotData = z.infer; +export type Bot = Omit; + +export type Services = z.infer; + +export type BotDataParams = { + realm_bots: ServerAddBotData[]; +}; + +const bots = new Map(); +const services = new Map(); + +// Define zod schema for data validation +const basic_bot_schema = z.object({ + api_key: z.string(), + avatar_url: z.string(), + bot_type: z.number(), + default_all_public_streams: z.boolean(), + default_events_register_stream: z.union([z.string(), z.null()]), + default_sending_stream: z.union([z.string(), z.null()]), + email: z.string(), + full_name: z.string(), + is_active: z.boolean(), + owner: z.union([z.string(), z.undefined()]), + owner_id: z.number(), + user_id: z.number(), +}); + +const outgoing_service_schema = z.object({ + base_url: z.string(), + interface: z.number(), + token: z.string(), +}); + +const embedded_service_schema = z.object({ + config_data: z.record(z.string()), + service_name: z.string(), +}); + +const services_schema = z.union([ + z.array(outgoing_service_schema), + z.array(embedded_service_schema), +]); + +const server_update_bot_schema = basic_bot_schema.extend({ + services: services_schema, +}); + +const server_add_bot_schema = server_update_bot_schema.extend({ + bot_type: z.number(), + email: z.string(), + is_active: z.boolean(), +}); + +export function all_user_ids(): number[] { + return [...bots.keys()]; +} + +export function add(bot_data: ServerAddBotData): void { + const {services: bot_services, ...clean_bot} = server_add_bot_schema.parse(bot_data); + bots.set(clean_bot.user_id, clean_bot); + + services.set(clean_bot.user_id, bot_services); +} + +export function deactivate(bot_id: number): void { + bots.get(bot_id)!.is_active = false; +} + +export function del(bot_id: number): void { + bots.delete(bot_id); + services.delete(bot_id); +} + +export function update(bot_id: number, bot_update: ServerUpdateBotData): void { + const bot = bots.get(bot_id)!; + Object.assign(bot, server_update_bot_schema.deepPartial().parse(bot_update)); + + // We currently only support one service per bot. + const service = services.get(bot_id)![0]; + if (bot_update.services !== undefined && bot_update.services.length > 0) { + Object.assign(service, services_schema.parse(bot_update.services)[0]); + } +} + +export function get_all_bots_for_current_user(): Bot[] { + const ret = []; + for (const bot of bots.values()) { + if (people.is_my_user_id(bot.owner_id)) { + ret.push(bot); + } + } + return ret; +} + +export function get_editable(): Bot[] { + const ret = []; + for (const bot of bots.values()) { + if (bot.is_active && people.is_my_user_id(bot.owner_id)) { + ret.push(bot); + } + } + return ret; +} + +export function get_all_bots_owned_by_user(user_id: number): Bot[] { + const ret = []; + for (const bot of bots.values()) { + if (bot.owner_id === user_id && bot.is_active) { + ret.push(bot); + } + } + return ret; +} + +export function get(bot_id: number): Bot | undefined { + return bots.get(bot_id); +} + +export function get_services(bot_id: number): Services | undefined { + return services.get(bot_id); +} + +export function initialize(params: BotDataParams): void { + bots.clear(); + for (const bot of params.realm_bots) { + add(bot); + } +} diff --git a/web/tests/bot_data.test.js b/web/tests/bot_data.test.js index 857c55f588..4b066fe853 100644 --- a/web/tests/bot_data.test.js +++ b/web/tests/bot_data.test.js @@ -9,6 +9,10 @@ const bot_data = zrequire("bot_data"); const people = zrequire("people"); +// Bot types and service bot types can be found +// in zerver/models.py - UserProfile Class or +// zever/openapi/zulip.yaml + const me = { email: "me@zulip.com", full_name: "Me Myself", @@ -23,12 +27,37 @@ const fred = { const bot_data_params = { realm_bots: [ - {email: "bot0@zulip.com", user_id: 42, full_name: "Bot 0", services: []}, { + api_key: "1234567890qwertyuioop", + avatar_url: "", + bot_type: 1, // DEFAULT_BOT + default_all_public_streams: true, + default_events_register_stream: "register stream 42", + default_sending_stream: "sending stream 42", + email: "bot0@zulip.com", + full_name: "Bot 0", + is_active: true, + owner: "someone 4", + owner_id: 4, + user_id: 42, + services: [], + extra: "This field should be ignored", + }, + { + api_key: "1234567890zxcvbnm", + avatar_url: "", + bot_type: 3, // OUTGOING_WEBHOOK_BOT + default_all_public_streams: true, + default_events_register_stream: "register stream 314", + default_sending_stream: "sending stream 314", email: "outgoingwebhook@zulip.com", - user_id: 314, full_name: "Outgoing webhook", - services: [{base_url: "http://foo.com", interface: 1}], + is_active: true, + owner: "someone 5", + owner_id: 5, + user_id: 314, + services: [{base_url: "http://foo.com", interface: 1, token: "basictoken12345"}], + extra: "This field should be ignored", }, ], }; @@ -48,31 +77,68 @@ function test(label, f) { test("test_basics", () => { people.add_active_user(fred); const test_bot = { + api_key: "qwertyuioop1234567890", + avatar_url: "", + bot_type: 1, + default_all_public_streams: true, + default_events_register_stream: "register stream 43", + default_sending_stream: "sending stream 43", email: "bot1@zulip.com", - user_id: 43, - avatar_url: "", full_name: "Bot 1", - services: [{base_url: "http://bar.com", interface: 1}], - extra: "Not in data", + is_active: true, + owner: "someone 6", + owner_id: 6, + user_id: 43, + services: [ + { + base_url: "http://bar.com", + interface: 1, + token: "some Bot 1 token", + }, + ], + extra: "This field should be ignored", }; - const test_embedded_bot = { - email: "embedded-bot@zulip.com", - user_id: 143, + api_key: "zxcvbnm1234567890", avatar_url: "", + bot_type: 4, // EMBEDDED_BOT + default_all_public_streams: true, + default_events_register_stream: "register stream 143", + default_sending_stream: "sending stream 143", + email: "embedded-bot@zulip.com", full_name: "Embedded bot 1", - services: [{config_data: {key: "12345678"}, service_name: "giphy"}], + is_active: true, owner: "cordelia@zulip.com", + owner_id: 7, + user_id: 143, + services: [ + { + config_data: {key: "12345678"}, + service_name: "giphy", + }, + ], + extra: "This field should be ignored", }; (function test_add() { bot_data.add(test_bot); - const bot = bot_data.get(43); const services = bot_data.get_services(43); + assert.equal("qwertyuioop1234567890", bot.api_key); + assert.equal("", bot.avatar_url); + assert.equal(1, bot.bot_type); + assert.equal(true, bot.default_all_public_streams); + assert.equal("register stream 43", bot.default_events_register_stream); + assert.equal("sending stream 43", bot.default_sending_stream); + assert.equal("bot1@zulip.com", bot.email); assert.equal("Bot 1", bot.full_name); + assert.equal(true, bot.is_active); + assert.equal("someone 6", bot.owner); + assert.equal(6, bot.owner_id); + assert.equal(43, bot.user_id); assert.equal("http://bar.com", services[0].base_url); assert.equal(1, services[0].interface); + assert.equal("some Bot 1 token", services[0].token); assert.equal(undefined, bot.extra); })(); @@ -82,19 +148,21 @@ test("test_basics", () => { let bot = bot_data.get(43); assert.equal("Bot 1", bot.full_name); bot_data.update(43, { + ...test_bot, full_name: "New Bot 1", - services: [{interface: 2, base_url: "http://baz.com"}], + services: [{interface: 2, base_url: "http://baz.com", token: "zxcvbnm1234567890"}], }); bot = bot_data.get(43); const services = bot_data.get_services(43); assert.equal("New Bot 1", bot.full_name); assert.equal(2, services[0].interface); assert.equal("http://baz.com", services[0].base_url); + assert.equal("zxcvbnm1234567890", services[0].token); const change_owner_event = { owner_id: fred.user_id, }; - bot_data.update(43, change_owner_event); + bot_data.update(43, {...test_bot, ...change_owner_event}); bot = bot_data.get(43); assert.equal(bot.owner_id, fred.user_id); @@ -105,8 +173,12 @@ test("test_basics", () => { const bot_id = 143; const services = bot_data.get_services(bot_id); assert.equal("12345678", services[0].config_data.key); - bot_data.update(bot_id, {services: [{config_data: {key: "87654321"}}]}); + bot_data.update(bot_id, { + ...test_embedded_bot, + services: [{config_data: {key: "87654321"}, service_name: "embedded bot service"}], + }); assert.equal("87654321", services[0].config_data.key); + assert.equal("embedded bot service", services[0].service_name); })(); (function test_remove() { diff --git a/web/tests/settings_bots.test.js b/web/tests/settings_bots.test.js index 1d1c81e7a0..c723961360 100644 --- a/web/tests/settings_bots.test.js +++ b/web/tests/settings_bots.test.js @@ -11,10 +11,19 @@ const bot_data_params = { realm_bots: [ { api_key: "QadL788EkiottHmukyhHgePUFHREiu8b", + avatar_url: "", + bot_type: 1, // DEFAULT_BOT + default_all_public_streams: true, + default_events_register_stream: "register stream 1", + default_sending_stream: "sending stream 1", email: "error-bot@zulip.org", full_name: "Error bot", + is_active: true, + owner: "someone 4", + owner_id: 4, user_id: 1, services: [], + extra: "This field should be ignored", }, ], };