From 23f21d74486db37e85e1f2eb20bbaf9704890c6e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Apr 2026 06:53:27 +0000 Subject: [PATCH] Fix code review issues: div-by-zero, duplicate logic, cron_name fallback Agent-Logs-Url: https://github.com/whyour/qinglong/sessions/3db54913-03d2-4721-b720-8ccbf8d0f00e Co-authored-by: whyour <22700758+whyour@users.noreply.github.com> --- back/services/cron.ts | 3 +- back/services/cronStats.ts | 155 ++++++++++++++------------------- src/pages/statistics/index.tsx | 2 +- 3 files changed, 67 insertions(+), 93 deletions(-) diff --git a/back/services/cron.ts b/back/services/cron.ts index 79224581..10770a88 100644 --- a/back/services/cron.ts +++ b/back/services/cron.ts @@ -179,10 +179,11 @@ export default class CronService { ); if (status === CrontabStatus.idle && last_running_time > 0) { + const cronName = (cron.name || cron.command || '').substring(0, 255); await CronLogModel.create( new CronLog({ cron_id: id, - cron_name: cron.name || cron.command || '', + cron_name: cronName, start_time: last_execution_time, duration: last_running_time, }), diff --git a/back/services/cronStats.ts b/back/services/cronStats.ts index 953e4481..f6f546b3 100644 --- a/back/services/cronStats.ts +++ b/back/services/cronStats.ts @@ -1,34 +1,64 @@ import { Service, Inject } from 'typedi'; import winston from 'winston'; -import { CrontabModel, CrontabStatus } from '../data/cron'; -import { CronLogModel } from '../data/cronLog'; +import { CrontabModel } from '../data/cron'; +import { CronLog, CronLogModel } from '../data/cronLog'; import { Op } from 'sequelize'; import dayjs from 'dayjs'; +type GroupedLog = { + cron_id: number; + cron_name: string; + durations: number[]; +}; + @Service() export default class CronStatsService { constructor(@Inject('logger') private logger: winston.Logger) {} + private groupLogsByCronId(logs: CronLog[]): Record { + const grouped: Record = {}; + for (const log of logs) { + if (!grouped[log.cron_id]) { + grouped[log.cron_id] = { + cron_id: log.cron_id, + cron_name: log.cron_name, + durations: [], + }; + } + grouped[log.cron_id].durations.push(log.duration); + } + return grouped; + } + + private avgOf(nums: number[]): number { + if (nums.length === 0) return 0; + return Math.round(nums.reduce((a, b) => a + b, 0) / nums.length); + } + + private getTodayRange() { + return { + start: dayjs().startOf('day').unix(), + end: dayjs().endOf('day').unix(), + }; + } + public async stats() { - const todayStart = dayjs().startOf('day').unix(); - const todayEnd = dayjs().endOf('day').unix(); + const { start, end } = this.getTodayRange(); const [allCrons, todayLogs] = await Promise.all([ CrontabModel.findAll({ where: {} }), CronLogModel.findAll({ - where: { - start_time: { [Op.between]: [todayStart, todayEnd] }, - }, + where: { start_time: { [Op.between]: [start, end] } }, }), ]); const total = allCrons.length; - const enabled = allCrons.filter((c) => c.isDisabled !== 1).length; - const disabled = allCrons.filter((c) => c.isDisabled === 1).length; + const enabled = allCrons.filter((c: any) => c.isDisabled !== 1).length; + const disabled = allCrons.filter((c: any) => c.isDisabled === 1).length; const todayCount = todayLogs.length; const todayTotalDuration = todayLogs.reduce( - (sum, l) => sum + (l.duration || 0), + (sum: number, l: any) => sum + (l.duration || 0), 0, ); const todayAvgDuration = @@ -47,10 +77,7 @@ export default class CronStatsService { public async trend() { const days = 7; - const result: Array<{ - date: string; - count: number; - }> = []; + const result: Array<{ date: string; count: number }> = []; for (let i = days - 1; i >= 0; i--) { const dayStart = dayjs().subtract(i, 'day').startOf('day').unix(); @@ -58,107 +85,53 @@ export default class CronStatsService { const date = dayjs().subtract(i, 'day').format('MM-DD'); const logs = await CronLogModel.findAll({ - where: { - start_time: { [Op.between]: [dayStart, dayEnd] }, - }, + where: { start_time: { [Op.between]: [dayStart, dayEnd] } }, }); - result.push({ - date, - count: logs.length, - }); + result.push({ date, count: logs.length }); } return result; } public async topDuration(limit = 5) { - const todayStart = dayjs().startOf('day').unix(); - const todayEnd = dayjs().endOf('day').unix(); + const { start, end } = this.getTodayRange(); const logs = await CronLogModel.findAll({ - where: { - start_time: { [Op.between]: [todayStart, todayEnd] }, - }, + where: { start_time: { [Op.between]: [start, end] } }, }); - const grouped: Record< - number, - { cron_id: number; cron_name: string; durations: number[] } - > = {}; + const grouped = this.groupLogsByCronId(logs as any); - for (const log of logs) { - if (!grouped[log.cron_id]) { - grouped[log.cron_id] = { - cron_id: log.cron_id, - cron_name: log.cron_name, - durations: [], - }; - } - grouped[log.cron_id].durations.push(log.duration); - } - - const result = Object.values(grouped) - .map((g) => { - const avgDuration = Math.round( - g.durations.reduce((a, b) => a + b, 0) / g.durations.length, - ); - const maxDuration = Math.max(...g.durations); - return { - cron_id: g.cron_id, - cron_name: g.cron_name, - count: g.durations.length, - avgDuration, - maxDuration, - }; - }) + return Object.values(grouped) + .map((g) => ({ + cron_id: g.cron_id, + cron_name: g.cron_name, + count: g.durations.length, + avgDuration: this.avgOf(g.durations), + maxDuration: Math.max(...g.durations), + })) .sort((a, b) => b.avgDuration - a.avgDuration) .slice(0, limit); - - return result; } public async topCount(limit = 5) { - const todayStart = dayjs().startOf('day').unix(); - const todayEnd = dayjs().endOf('day').unix(); + const { start, end } = this.getTodayRange(); const logs = await CronLogModel.findAll({ - where: { - start_time: { [Op.between]: [todayStart, todayEnd] }, - }, + where: { start_time: { [Op.between]: [start, end] } }, }); - const grouped: Record< - number, - { cron_id: number; cron_name: string; durations: number[] } - > = {}; + const grouped = this.groupLogsByCronId(logs as any); - for (const log of logs) { - if (!grouped[log.cron_id]) { - grouped[log.cron_id] = { - cron_id: log.cron_id, - cron_name: log.cron_name, - durations: [], - }; - } - grouped[log.cron_id].durations.push(log.duration); - } - - const result = Object.values(grouped) - .map((g) => { - const avgDuration = Math.round( - g.durations.reduce((a, b) => a + b, 0) / g.durations.length, - ); - return { - cron_id: g.cron_id, - cron_name: g.cron_name, - count: g.durations.length, - avgDuration, - }; - }) + return Object.values(grouped) + .map((g) => ({ + cron_id: g.cron_id, + cron_name: g.cron_name, + count: g.durations.length, + avgDuration: this.avgOf(g.durations), + })) .sort((a, b) => b.count - a.count) .slice(0, limit); - - return result; } } diff --git a/src/pages/statistics/index.tsx b/src/pages/statistics/index.tsx index fe1a8a0c..6c49abfa 100644 --- a/src/pages/statistics/index.tsx +++ b/src/pages/statistics/index.tsx @@ -73,7 +73,7 @@ const TrendChart = ({ data }: { data: TrendItem[] }) => { const maxCount = Math.max(...data.map((d) => d.count), 1); const points = data.map((d, i) => ({ - x: paddingLeft + (i / (data.length - 1)) * chartWidth, + x: paddingLeft + (i / Math.max(data.length - 1, 1)) * chartWidth, y: paddingTop + chartHeight - (d.count / maxCount) * chartHeight, ...d, }));