From e2669af87021dfa86a383468382f3d511c82a3ed Mon Sep 17 00:00:00 2001 From: Misaka Date: Sun, 5 Apr 2026 12:15:18 +0800 Subject: [PATCH] fix: remove unused imports and fix logger test isolation - Remove unused imports (run, trackDuration, PerformanceTracker, ConfigManager, disconnectDb) flagged by ESLint - Remove unused isSlow variable in performance-monitor catch block - Add eslint-disable for require() in Playwright JS script - Fix logger-performance test flakiness by using vi.resetModules() with dynamic imports to prevent cached logger references across test files Co-Authored-By: Claude Opus 4.6 --- src/main/services/config/config-manager.ts | 11 +- src/main/services/database/data-source.ts | 9 +- .../database/discrete-material-plan-dao.ts | 2 +- .../extractor-operation-history-dao.ts | 4 +- .../database/materials-to-be-deleted-dao.ts | 2 +- .../materials-type-to-be-deleted-dao.ts | 2 +- src/main/services/database/postgresql.ts | 128 +++++++++++++++--- .../services/logger/performance-monitor.ts | 1 - src/main/services/rustfs/rustfs-service.ts | 2 +- src/main/services/update/update-service.ts | 2 +- src/main/services/user/bip-users-dao.ts | 7 +- tests/integration/logger-performance.test.ts | 40 +++--- tests/playwright/verify_report_analysis.js | 1 + tests/unit/postgresql.test.ts | 12 +- 14 files changed, 157 insertions(+), 66 deletions(-) diff --git a/src/main/services/config/config-manager.ts b/src/main/services/config/config-manager.ts index 4af93b8..5b8b861 100644 --- a/src/main/services/config/config-manager.ts +++ b/src/main/services/config/config-manager.ts @@ -20,7 +20,7 @@ import { dirname } from 'path' import { app } from 'electron' import yaml from 'js-yaml' import { z } from 'zod' -import { createLogger, applyLoggingConfig, trackDuration } from '../logger' +import { createLogger, applyLoggingConfig } from '../logger' import { applyAuditConfig } from '../logger/audit-logger' import { fullConfigSchema, @@ -315,9 +315,12 @@ export class ConfigManager { const { activeType, mysql, sqlserver, postgresql } = this.config.database switch (activeType) { - case 'postgresql': return postgresql - case 'sqlserver': return sqlserver - default: return mysql + case 'postgresql': + return postgresql + case 'sqlserver': + return sqlserver + default: + return mysql } } diff --git a/src/main/services/database/data-source.ts b/src/main/services/database/data-source.ts index ada5be6..6840c52 100644 --- a/src/main/services/database/data-source.ts +++ b/src/main/services/database/data-source.ts @@ -22,9 +22,12 @@ function getDatabaseType(): 'mysql' | 'mssql' | 'postgres' { const configManager = ConfigManager.getInstance() const dbType = configManager.getDatabaseType() switch (dbType) { - case 'sqlserver': return 'mssql' - case 'postgresql': return 'postgres' - default: return 'mysql' + case 'sqlserver': + return 'mssql' + case 'postgresql': + return 'postgres' + default: + return 'mysql' } } diff --git a/src/main/services/database/discrete-material-plan-dao.ts b/src/main/services/database/discrete-material-plan-dao.ts index 9dd5110..af40581 100644 --- a/src/main/services/database/discrete-material-plan-dao.ts +++ b/src/main/services/database/discrete-material-plan-dao.ts @@ -10,7 +10,7 @@ import { create, type IDatabaseService } from './index' import { createDialect, type SqlDialect } from './dialects' -import { createLogger, run, getRequestId, trackDuration } from '../logger' +import { createLogger, getRequestId, trackDuration } from '../logger' const log = createLogger('DiscreteMaterialPlanDAO') diff --git a/src/main/services/database/extractor-operation-history-dao.ts b/src/main/services/database/extractor-operation-history-dao.ts index 4192598..9893f47 100644 --- a/src/main/services/database/extractor-operation-history-dao.ts +++ b/src/main/services/database/extractor-operation-history-dao.ts @@ -11,7 +11,7 @@ import { create, type IDatabaseService } from './index' import { createDialect, type SqlDialect } from './dialects' -import { createLogger, run, getRequestId, trackDuration } from '../logger' +import { createLogger, getRequestId, trackDuration } from '../logger' import type { OperationHistoryRecord, BatchStats, @@ -202,7 +202,7 @@ export class ExtractorOperationHistoryDAO { ` const params = [status, batchId] - const result = await trackDuration(async () => await dbService.query(sqlString, params), { + await trackDuration(async () => await dbService.query(sqlString, params), { operationName: 'ExtractorOperationHistoryDAO.updateBatchStatus', context: { tableName, operationType: 'UPDATE', batchId } }) diff --git a/src/main/services/database/materials-to-be-deleted-dao.ts b/src/main/services/database/materials-to-be-deleted-dao.ts index b6f89b3..c9ce34e 100644 --- a/src/main/services/database/materials-to-be-deleted-dao.ts +++ b/src/main/services/database/materials-to-be-deleted-dao.ts @@ -10,7 +10,7 @@ import { create, type IDatabaseService } from './index' import { createDialect, type SqlDialect } from './dialects' -import { createLogger, run, getRequestId, trackDuration } from '../logger' +import { createLogger, getRequestId, trackDuration } from '../logger' const log = createLogger('MaterialsToBeDeletedDAO') diff --git a/src/main/services/database/materials-type-to-be-deleted-dao.ts b/src/main/services/database/materials-type-to-be-deleted-dao.ts index eff9c67..e48a5e0 100644 --- a/src/main/services/database/materials-type-to-be-deleted-dao.ts +++ b/src/main/services/database/materials-type-to-be-deleted-dao.ts @@ -7,7 +7,7 @@ import { create, type IDatabaseService } from './index' import { createDialect, type SqlDialect } from './dialects' -import { createLogger, run, getRequestId, trackDuration } from '../logger' +import { createLogger, getRequestId, trackDuration } from '../logger' const log = createLogger('MaterialsTypeToBeDeletedDAO') diff --git a/src/main/services/database/postgresql.ts b/src/main/services/database/postgresql.ts index b0eba23..57bf6e9 100644 --- a/src/main/services/database/postgresql.ts +++ b/src/main/services/database/postgresql.ts @@ -18,37 +18,123 @@ export type { PostgreSqlConfig } from '../../types/database.types' */ const SQL_KEYWORDS = new Set([ // DML - 'SELECT', 'FROM', 'WHERE', 'AND', 'OR', 'NOT', 'IN', 'IS', 'NULL', - 'INSERT', 'INTO', 'VALUES', 'UPDATE', 'SET', 'DELETE', + 'SELECT', + 'FROM', + 'WHERE', + 'AND', + 'OR', + 'NOT', + 'IN', + 'IS', + 'NULL', + 'INSERT', + 'INTO', + 'VALUES', + 'UPDATE', + 'SET', + 'DELETE', // Ordering & limiting - 'ORDER', 'BY', 'ASC', 'DESC', 'LIMIT', 'OFFSET', - 'FETCH', 'NEXT', 'ROWS', 'ONLY', + 'ORDER', + 'BY', + 'ASC', + 'DESC', + 'LIMIT', + 'OFFSET', + 'FETCH', + 'NEXT', + 'ROWS', + 'ONLY', // Joins - 'JOIN', 'LEFT', 'RIGHT', 'INNER', 'OUTER', 'CROSS', 'FULL', 'ON', + 'JOIN', + 'LEFT', + 'RIGHT', + 'INNER', + 'OUTER', + 'CROSS', + 'FULL', + 'ON', // Set operations - 'UNION', 'ALL', 'INTERSECT', 'EXCEPT', + 'UNION', + 'ALL', + 'INTERSECT', + 'EXCEPT', // Grouping - 'GROUP', 'HAVING', 'DISTINCT', + 'GROUP', + 'HAVING', + 'DISTINCT', // DDL - 'CREATE', 'ALTER', 'DROP', 'TABLE', 'INDEX', 'COLUMN', - 'ADD', 'MODIFY', 'RENAME', 'TO', + 'CREATE', + 'ALTER', + 'DROP', + 'TABLE', + 'INDEX', + 'COLUMN', + 'ADD', + 'MODIFY', + 'RENAME', + 'TO', // PostgreSQL specific - 'CONFLICT', 'DO', 'NOTHING', 'EXCLUDED', 'RETURNING', - 'MERGE', 'USING', 'MATCHED', 'WHEN', 'THEN', 'ELSE', 'END', - 'TARGET', 'SOURCE', + 'CONFLICT', + 'DO', + 'NOTHING', + 'EXCLUDED', + 'RETURNING', + 'MERGE', + 'USING', + 'MATCHED', + 'WHEN', + 'THEN', + 'ELSE', + 'END', + 'TARGET', + 'SOURCE', // Functions - 'COUNT', 'SUM', 'AVG', 'MIN', 'MAX', 'EXISTS', - 'CURRENT_TIMESTAMP', 'NOW', 'GETDATE', - 'COALESCE', 'NULLIF', 'CAST', 'AS', + 'COUNT', + 'SUM', + 'AVG', + 'MIN', + 'MAX', + 'EXISTS', + 'CURRENT_TIMESTAMP', + 'NOW', + 'GETDATE', + 'COALESCE', + 'NULLIF', + 'CAST', + 'AS', // Transaction - 'BEGIN', 'COMMIT', 'ROLLBACK', 'SAVEPOINT', + 'BEGIN', + 'COMMIT', + 'ROLLBACK', + 'SAVEPOINT', // Types & values - 'TRUE', 'FALSE', 'DEFAULT', 'PRIMARY', 'KEY', - 'REFERENCES', 'FOREIGN', 'CONSTRAINT', 'UNIQUE', 'CHECK', - 'CASE', 'BETWEEN', 'LIKE', 'ILIKE', 'ANY', 'SOME', + 'TRUE', + 'FALSE', + 'DEFAULT', + 'PRIMARY', + 'KEY', + 'REFERENCES', + 'FOREIGN', + 'CONSTRAINT', + 'UNIQUE', + 'CHECK', + 'CASE', + 'BETWEEN', + 'LIKE', + 'ILIKE', + 'ANY', + 'SOME', // Common - 'IF', 'WITH', 'RECURSIVE', 'OVER', 'PARTITION', 'WINDOW', - 'ROW', 'FIRST', 'AFTER', 'BEFORE' + 'IF', + 'WITH', + 'RECURSIVE', + 'OVER', + 'PARTITION', + 'WINDOW', + 'ROW', + 'FIRST', + 'AFTER', + 'BEFORE' ]) /** diff --git a/src/main/services/logger/performance-monitor.ts b/src/main/services/logger/performance-monitor.ts index 9f81886..51e794d 100644 --- a/src/main/services/logger/performance-monitor.ts +++ b/src/main/services/logger/performance-monitor.ts @@ -119,7 +119,6 @@ export async function trackDuration( return { result, durationMs, isSlow } } catch (error) { const durationMs = performance.now() - startTime - const isSlow = durationMs > slowThresholdMs // Log the error with duration logger.error(`${message} failed after ${durationMs.toFixed(2)}ms`, { diff --git a/src/main/services/rustfs/rustfs-service.ts b/src/main/services/rustfs/rustfs-service.ts index bd5938a..5e1391f 100644 --- a/src/main/services/rustfs/rustfs-service.ts +++ b/src/main/services/rustfs/rustfs-service.ts @@ -15,7 +15,7 @@ import { type GetObjectCommandInput, type DeleteObjectCommandInput } from '@aws-sdk/client-s3' -import { createLogger, run, trackDuration, PerformanceTracker } from '../logger' +import { createLogger } from '../logger' import type { RustfsConfig } from '../../types/config.schema' import * as fs from 'fs' import * as path from 'path' diff --git a/src/main/services/update/update-service.ts b/src/main/services/update/update-service.ts index 461c6df..fc6ce4e 100644 --- a/src/main/services/update/update-service.ts +++ b/src/main/services/update/update-service.ts @@ -1,6 +1,6 @@ import * as fs from 'fs' import { ConfigManager } from '../config/config-manager' -import { createLogger, run, trackDuration, PerformanceTracker } from '../logger' +import { createLogger } from '../logger' import type { UpdateConfig } from '../../types/config.schema' import type { UserType } from '../../types/user.types' import type { diff --git a/src/main/services/user/bip-users-dao.ts b/src/main/services/user/bip-users-dao.ts index ae4c727..a5667a7 100644 --- a/src/main/services/user/bip-users-dao.ts +++ b/src/main/services/user/bip-users-dao.ts @@ -8,13 +8,8 @@ * - Create, update, delete users */ -import { - create, - disconnect as disconnectDb, - type IDatabaseService -} from '../database/index' +import { create, type IDatabaseService } from '../database/index' import { createDialect, type SqlDialect } from '../database/dialects' -import { ConfigManager } from '../config/config-manager' import type { UserInfo } from '../../types/user.types' import { createLogger, logError } from '../logger' diff --git a/tests/integration/logger-performance.test.ts b/tests/integration/logger-performance.test.ts index 1303817..f6727f6 100644 --- a/tests/integration/logger-performance.test.ts +++ b/tests/integration/logger-performance.test.ts @@ -5,14 +5,6 @@ */ import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest' -import { - trackDuration, - PerformanceTracker, - createPerformanceTracker, - DEFAULT_SLOW_THRESHOLD_MS, - type TrackDurationOptions -} from '../../src/main/services/logger/performance-monitor' -import logger from '../../src/main/services/logger/index' // Mock the logger to avoid noisy output during tests vi.mock('../../src/main/services/logger', () => ({ @@ -26,7 +18,23 @@ vi.mock('../../src/main/services/logger', () => ({ })) describe('Performance Monitor', () => { - beforeEach(() => { + let trackDuration: typeof import('../../src/main/services/logger/performance-monitor').trackDuration + let PerformanceTracker: typeof import('../../src/main/services/logger/performance-monitor').PerformanceTracker + let createPerformanceTracker: typeof import('../../src/main/services/logger/performance-monitor').createPerformanceTracker + let DEFAULT_SLOW_THRESHOLD_MS: typeof import('../../src/main/services/logger/performance-monitor').DEFAULT_SLOW_THRESHOLD_MS + let logger: typeof import('../../src/main/services/logger').default + + beforeEach(async () => { + vi.resetModules() + const perfMod = await import('../../src/main/services/logger/performance-monitor') + const loggerMod = await import('../../src/main/services/logger/index') + + trackDuration = perfMod.trackDuration + PerformanceTracker = perfMod.PerformanceTracker + createPerformanceTracker = perfMod.createPerformanceTracker + DEFAULT_SLOW_THRESHOLD_MS = perfMod.DEFAULT_SLOW_THRESHOLD_MS + logger = loggerMod.default + vi.clearAllMocks() }) @@ -230,7 +238,7 @@ describe('Performance Monitor', () => { }) }) - it('should log summary with aggregated metrics', () => { + it('should log summary with aggregated metrics', async () => { const tracker = new PerformanceTracker('TestService', 1000) tracker.recordDuration(100) @@ -248,7 +256,7 @@ describe('Performance Monitor', () => { }) }) - it('should include slow percentage in summary', () => { + it('should include slow percentage in summary', async () => { const tracker = new PerformanceTracker('TestService', 50) tracker.recordDuration(30) // Normal @@ -261,7 +269,7 @@ describe('Performance Monitor', () => { expect(summaryCall.slowPercentage).toContain('%') }) - it('should reset metrics when reset() is called', () => { + it('should reset metrics when reset() is called', async () => { const tracker = new PerformanceTracker('TestService', 1000) tracker.recordDuration(100) @@ -275,7 +283,7 @@ describe('Performance Monitor', () => { expect(metrics.slowOperationCount).toBe(0) }) - it('should return zero metrics when no operations tracked', () => { + it('should return zero metrics when no operations tracked', async () => { const tracker = new PerformanceTracker('EmptyService') const metrics = tracker.getMetrics() @@ -288,7 +296,7 @@ describe('Performance Monitor', () => { expect(metrics.slowOperationCount).toBe(0) }) - it('should use custom logger if provided', () => { + it('should use custom logger if provided', async () => { const customLogger = { debug: vi.fn(), info: vi.fn(), @@ -307,7 +315,7 @@ describe('Performance Monitor', () => { }) describe('createPerformanceTracker', () => { - it('should create a tracker with default threshold', () => { + it('should create a tracker with default threshold', async () => { const tracker = createPerformanceTracker('MyService') expect(tracker).toBeInstanceOf(PerformanceTracker) @@ -315,7 +323,7 @@ describe('Performance Monitor', () => { expect(metrics.count).toBe(0) }) - it('should create a tracker with custom threshold', () => { + it('should create a tracker with custom threshold', async () => { const tracker = createPerformanceTracker('FastService', 100) tracker.recordDuration(150) diff --git a/tests/playwright/verify_report_analysis.js b/tests/playwright/verify_report_analysis.js index deefdb4..d2db2ad 100644 --- a/tests/playwright/verify_report_analysis.js +++ b/tests/playwright/verify_report_analysis.js @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-require-imports */ const { _electron: electron } = require('playwright') ;(async () => { diff --git a/tests/unit/postgresql.test.ts b/tests/unit/postgresql.test.ts index 8b6d9c5..fe18f7b 100644 --- a/tests/unit/postgresql.test.ts +++ b/tests/unit/postgresql.test.ts @@ -132,7 +132,7 @@ describe('prepareSql', () => { it('should preserve string literals with escaped quotes', () => { const sql = "WHERE UserName = 'O''Brien'" const result = prepareSql(sql) - expect(result).toBe('WHERE "UserName" = \'O\'\'Brien\'') + expect(result).toBe("WHERE \"UserName\" = 'O''Brien'") }) it('should preserve $N parameter placeholders', () => { @@ -145,17 +145,13 @@ describe('prepareSql', () => { it('should handle COUNT(*) correctly', () => { const sql = 'SELECT COUNT(*) as count FROM "dbo"."BIPUsers" WHERE UserName = $1' const result = prepareSql(sql) - expect(result).toBe( - 'SELECT COUNT(*) as count FROM "dbo"."BIPUsers" WHERE "UserName" = $1' - ) + expect(result).toBe('SELECT COUNT(*) as count FROM "dbo"."BIPUsers" WHERE "UserName" = $1') }) it('should quote underscore-containing column names', () => { const sql = 'SELECT ERP_URL, ERP_Username, ERP_Password FROM "dbo"."BIPUsers"' const result = prepareSql(sql) - expect(result).toBe( - 'SELECT "ERP_URL", "ERP_Username", "ERP_Password" FROM "dbo"."BIPUsers"' - ) + expect(result).toBe('SELECT "ERP_URL", "ERP_Username", "ERP_Password" FROM "dbo"."BIPUsers"') }) it('should handle ON CONFLICT DO UPDATE SET with EXCLUDED', () => { @@ -168,7 +164,7 @@ describe('prepareSql', () => { }) it('should handle CURRENT_TIMESTAMP without quoting', () => { - const sql = "INSERT INTO t (OperationTime) VALUES (CURRENT_TIMESTAMP)" + const sql = 'INSERT INTO t (OperationTime) VALUES (CURRENT_TIMESTAMP)' const result = prepareSql(sql) expect(result).toBe('INSERT INTO "t" ("OperationTime") VALUES (CURRENT_TIMESTAMP)') })