From 166c6f7da067d8f29ec879ddaa697f78d2f94eb4 Mon Sep 17 00:00:00 2001 From: Xiao Yijun Date: Wed, 24 May 2023 08:28:39 +0800 Subject: [PATCH] feat(core): get hook execution stats (#3882) --- packages/core/src/libraries/hook.test.ts | 15 ++++++- packages/core/src/libraries/hook.ts | 14 +++++- packages/core/src/queries/log.ts | 15 ++++++- packages/core/src/routes/hook.test.ts | 44 ++++++++++++++++++- packages/core/src/routes/hook.ts | 34 +++++++++++--- .../src/tests/api/hooks.test.ts | 44 ++++++++++++++++++- packages/schemas/src/types/hook.ts | 17 ++++++- 7 files changed, 168 insertions(+), 15 deletions(-) diff --git a/packages/core/src/libraries/hook.test.ts b/packages/core/src/libraries/hook.test.ts index 7652cf35a..42f03a429 100644 --- a/packages/core/src/libraries/hook.test.ts +++ b/packages/core/src/libraries/hook.test.ts @@ -3,6 +3,8 @@ import { HookEvent, InteractionEvent, LogResult } from '@logto/schemas'; import { createMockUtils } from '@logto/shared/esm'; import { got } from 'got'; +import { mockHook } from '#src/__mocks__/hook.js'; + import type { Interaction } from './hook.js'; const { jest } = import.meta; @@ -41,10 +43,12 @@ const post = jest .mockImplementation(jest.fn(async () => ({ statusCode: 200, body: '{"message":"ok"}' }))); const insertLog = jest.fn(); +const mockHookState = { requestCount: 100, successCount: 10 }; +const getHookExecutionStatsByHookId = jest.fn().mockResolvedValue(mockHookState); const findAllHooks = jest.fn().mockResolvedValue([hook]); const { createHookLibrary } = await import('./hook.js'); -const { triggerInteractionHooksIfNeeded } = createHookLibrary( +const { triggerInteractionHooksIfNeeded, attachExecutionStatsToHook } = createHookLibrary( new MockQueries({ // @ts-expect-error users: { findUserById: () => ({ id: 'user_id', username: 'user', extraField: 'not_ok' }) }, @@ -52,7 +56,7 @@ const { triggerInteractionHooksIfNeeded } = createHookLibrary( // @ts-expect-error findApplicationById: async () => ({ id: 'app_id', extraField: 'not_ok' }), }, - logs: { insertLog }, + logs: { insertLog, getHookExecutionStatsByHookId }, hooks: { findAllHooks }, }) ); @@ -116,3 +120,10 @@ describe('triggerInteractionHooksIfNeeded()', () => { jest.useRealTimers(); }); }); + +describe('attachExecutionStatsToHook', () => { + it('should attach execution stats to a hook', async () => { + const result = await attachExecutionStatsToHook(mockHook); + expect(result).toEqual({ ...mockHook, executionStats: mockHookState }); + }); +}); diff --git a/packages/core/src/libraries/hook.ts b/packages/core/src/libraries/hook.ts index 47d7aedde..e566861c5 100644 --- a/packages/core/src/libraries/hook.ts +++ b/packages/core/src/libraries/hook.ts @@ -4,6 +4,8 @@ import { InteractionEvent, LogResult, userInfoSelectFields, + type Hook, + type HookResponse, } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; import { conditional, pick, trySafe } from '@silverhand/essentials'; @@ -33,7 +35,7 @@ export type Interaction = Awaited>; export const createHookLibrary = (queries: Queries) => { const { applications: { findApplicationById }, - logs: { insertLog }, + logs: { insertLog, getHookExecutionStatsByHookId }, // TODO: @gao should we use the library function thus we can pass full userinfo to the payload? users: { findUserById }, hooks: { findAllHooks }, @@ -127,5 +129,13 @@ export const createHookLibrary = (queries: Queries) => { ); }; - return { triggerInteractionHooksIfNeeded }; + const attachExecutionStatsToHook = async (hook: Hook): Promise => ({ + ...hook, + executionStats: await getHookExecutionStatsByHookId(hook.id), + }); + + return { + triggerInteractionHooksIfNeeded, + attachExecutionStatsToHook, + }; }; diff --git a/packages/core/src/queries/log.ts b/packages/core/src/queries/log.ts index 8d7af358a..485c05a38 100644 --- a/packages/core/src/queries/log.ts +++ b/packages/core/src/queries/log.ts @@ -1,6 +1,7 @@ -import type { Log } from '@logto/schemas'; +import type { HookExecutionStats, Log } from '@logto/schemas'; import { token, Logs } from '@logto/schemas'; import { conditionalSql, convertToIdentifiers } from '@logto/shared'; +import { subDays } from 'date-fns'; import type { CommonQueryMethods } from 'slonik'; import { sql } from 'slonik'; @@ -86,6 +87,17 @@ export const createLogQueries = (pool: CommonQueryMethods) => { and ${fields.payload}->>'result' = 'Success' `); + const getHookExecutionStatsByHookId = async (hookId: string) => { + const startTimeExclusive = subDays(new Date(), 1).getTime(); + return pool.one(sql` + select count(*) as request_count, + count(case when ${fields.payload}->>'result' = 'Success' then 1 end) as success_count + from ${table} + where ${fields.createdAt} > to_timestamp(${startTimeExclusive}::double precision / 1000) + and ${fields.payload}->>'hookId' = ${hookId} + `); + }; + return { insertLog, countLogs, @@ -93,5 +105,6 @@ export const createLogQueries = (pool: CommonQueryMethods) => { findLogById, getDailyActiveUserCountsByTimeInterval, countActiveUsersByTimeInterval, + getHookExecutionStatsByHookId, }; }; diff --git a/packages/core/src/routes/hook.test.ts b/packages/core/src/routes/hook.test.ts index b797aec99..d4bf48589 100644 --- a/packages/core/src/routes/hook.test.ts +++ b/packages/core/src/routes/hook.test.ts @@ -53,6 +53,11 @@ const mockLog: Log = { createdAt: 123, }; +const mockExecutionStats = { + requestCount: 1, + successCount: 1, +}; + const logs = { countLogs: jest.fn().mockResolvedValue({ count: 1, @@ -62,7 +67,23 @@ const logs = { const { countLogs, findLogs } = logs; -const tenantContext = new MockTenant(undefined, { hooks, logs }); +const mockQueries = { + hooks, + logs, +}; + +const attachExecutionStatsToHook = jest.fn().mockImplementation((hook) => ({ + ...hook, + executionStats: mockExecutionStats, +})); + +const mockLibraries = { + hooks: { + attachExecutionStatsToHook, + }, +}; + +const tenantContext = new MockTenant(undefined, mockQueries, undefined, mockLibraries); const hookRoutes = await pickDefault(import('./hook.js')); @@ -80,6 +101,17 @@ describe('hook routes', () => { expect(response.header).not.toHaveProperty('total-number'); }); + it('GET /hooks?includeExecutionStats', async () => { + const response = await hookRequest.get('/hooks?includeExecutionStats=true'); + expect(attachExecutionStatsToHook).toHaveBeenCalledTimes(mockHookList.length); + expect(response.body).toEqual( + mockHookList.map((hook) => ({ + ...hook, + executionStats: mockExecutionStats, + })) + ); + }); + it('GET /hooks/:id', async () => { const hookIdInMockList = mockHookList[0]?.id ?? ''; const response = await hookRequest.get(`/hooks/${hookIdInMockList}`); @@ -87,6 +119,16 @@ describe('hook routes', () => { expect(response.body.id).toBe(hookIdInMockList); }); + it('GET /hooks/:id?includeExecutionStats', async () => { + const hookIdInMockList = mockHookList[0]?.id ?? ''; + const response = await hookRequest.get(`/hooks/${hookIdInMockList}?includeExecutionStats=true`); + expect(attachExecutionStatsToHook).toHaveBeenCalledWith(mockHookList[0]); + expect(response.body).toEqual({ + ...mockHookList[0], + executionStats: mockExecutionStats, + }); + }); + it('GET /hooks/:id/recent-logs should call countLogs and findLogs with correct parameters', async () => { jest.useFakeTimers().setSystemTime(100_000); diff --git a/packages/core/src/routes/hook.ts b/packages/core/src/routes/hook.ts index 5eea2e6bc..81fcd0730 100644 --- a/packages/core/src/routes/hook.ts +++ b/packages/core/src/routes/hook.ts @@ -1,6 +1,6 @@ -import { Hooks, Logs, hookEventsGuard } from '@logto/schemas'; +import { Hooks, Logs, hookEventsGuard, hookResponseGuard } from '@logto/schemas'; import { generateStandardId } from '@logto/shared'; -import { conditional, deduplicate } from '@silverhand/essentials'; +import { conditional, deduplicate, yes } from '@silverhand/essentials'; import { subDays } from 'date-fns'; import { z } from 'zod'; @@ -16,18 +16,34 @@ const nonemptyUniqueHookEventsGuard = hookEventsGuard .transform((events) => deduplicate(events)); export default function hookRoutes( - ...[router, { queries }]: RouterInitArgs + ...[router, { queries, libraries }]: RouterInitArgs ) { const { hooks: { findAllHooks, findHookById, insertHook, updateHookById, deleteHookById }, logs: { countLogs, findLogs }, } = queries; + const { + hooks: { attachExecutionStatsToHook }, + } = libraries; + router.get( '/hooks', - koaGuard({ response: Hooks.guard.array(), status: 200 }), + koaGuard({ + query: z.object({ includeExecutionStats: z.string().optional() }), + response: hookResponseGuard.partial({ executionStats: true }).array(), + status: 200, + }), async (ctx, next) => { - ctx.body = await findAllHooks(); + const { + query: { includeExecutionStats }, + } = ctx.guard; + + const hooks = await findAllHooks(); + + ctx.body = yes(includeExecutionStats) + ? await Promise.all(hooks.map(async (hook) => attachExecutionStatsToHook(hook))) + : hooks; return next(); } @@ -37,15 +53,19 @@ export default function hookRoutes( '/hooks/:id', koaGuard({ params: z.object({ id: z.string() }), - response: Hooks.guard, + query: z.object({ includeExecutionStats: z.string().optional() }), + response: hookResponseGuard.partial({ executionStats: true }), status: [200, 404], }), async (ctx, next) => { const { params: { id }, + query: { includeExecutionStats }, } = ctx.guard; - ctx.body = await findHookById(id); + const hook = await findHookById(id); + + ctx.body = includeExecutionStats ? await attachExecutionStatsToHook(hook) : hook; return next(); } diff --git a/packages/integration-tests/src/tests/api/hooks.test.ts b/packages/integration-tests/src/tests/api/hooks.test.ts index 439a7bb4f..c1704ac0e 100644 --- a/packages/integration-tests/src/tests/api/hooks.test.ts +++ b/packages/integration-tests/src/tests/api/hooks.test.ts @@ -1,7 +1,7 @@ import { createHmac } from 'node:crypto'; import { type RequestListener } from 'node:http'; -import type { Hook, HookConfig, Log, LogKey } from '@logto/schemas'; +import type { Hook, HookConfig, HookResponse, Log, LogKey } from '@logto/schemas'; import { HookEvent, SignInIdentifier, LogResult, InteractionEvent } from '@logto/schemas'; import { authedAdminApi, deleteUser, getLogs, putInteraction } from '#src/api/index.js'; @@ -336,4 +336,46 @@ describe('hooks', () => { await deleteUser(id); }); + + it('should get hook execution stats correctly', async () => { + const createdHook = await authedAdminApi + .post('hooks', { json: createPayload(HookEvent.PostRegister, 'http://localhost:9999') }) + .json(); + + const hooksWithExecutionStats = await authedAdminApi + .get('hooks?includeExecutionStats=true') + .json(); + + for (const hook of hooksWithExecutionStats) { + expect(hook.executionStats).toBeTruthy(); + } + + // Init session and submit + const { username, password } = generateNewUserProfile({ username: true, password: true }); + const client = await initClient(); + await client.send(putInteraction, { + event: InteractionEvent.Register, + profile: { + username, + password, + }, + }); + const { redirectTo } = await client.submitInteraction(); + const id = await processSession(client, redirectTo); + await waitFor(500); // Wait for hooks execution + + const hookWithExecutionStats = await authedAdminApi + .get(`hooks/${createdHook.id}?includeExecutionStats=true`) + .json(); + + const { executionStats } = hookWithExecutionStats; + + expect(executionStats).toBeTruthy(); + expect(executionStats.requestCount).toBe(1); + expect(executionStats.successCount).toBe(1); + + await authedAdminApi.delete(`hooks/${createdHook.id}`); + + await deleteUser(id); + }); }); diff --git a/packages/schemas/src/types/hook.ts b/packages/schemas/src/types/hook.ts index 2cec796a1..04b28f324 100644 --- a/packages/schemas/src/types/hook.ts +++ b/packages/schemas/src/types/hook.ts @@ -1,4 +1,6 @@ -import { type Application, type User } from '../db-entries/index.js'; +import { z } from 'zod'; + +import { Hooks, type Application, type User } from '../db-entries/index.js'; import { type HookEvent } from '../foundations/index.js'; import type { userInfoSelectFields } from './user.js'; @@ -13,3 +15,16 @@ export type HookEventPayload = { user?: Pick; application?: Pick; } & Record; + +const hookExecutionStatsGuard = z.object({ + successCount: z.number(), + requestCount: z.number(), +}); + +export type HookExecutionStats = z.infer; + +export const hookResponseGuard = Hooks.guard.extend({ + executionStats: hookExecutionStatsGuard, +}); + +export type HookResponse = z.infer;