From 7f7189d1d0941d161cdad398ba2caa07378787ed Mon Sep 17 00:00:00 2001 From: Jin Yi Date: Sun, 7 Sep 2025 19:14:00 +0900 Subject: [PATCH] refactor: namaing function modified --- .../dialog/footer/ManagerProgressFooter.vue | 4 +- src/composables/useConflictDetection.ts | 94 +++++++++---------- src/types/conflictDetectionTypes.ts | 2 +- .../footer/ManagerProgressFooter.test.ts | 2 +- .../composables/useConflictDetection.test.ts | 80 ++++++++-------- 5 files changed, 88 insertions(+), 94 deletions(-) diff --git a/src/components/dialog/footer/ManagerProgressFooter.vue b/src/components/dialog/footer/ManagerProgressFooter.vue index 5fb225f8b..db76c3884 100644 --- a/src/components/dialog/footer/ManagerProgressFooter.vue +++ b/src/components/dialog/footer/ManagerProgressFooter.vue @@ -91,7 +91,7 @@ const dialogStore = useDialogStore() const progressDialogContent = useManagerProgressDialogStore() const comfyManagerStore = useComfyManagerStore() const settingStore = useSettingStore() -const { performConflictDetection } = useConflictDetection() +const { runFullConflictAnalysis } = useConflictDetection() // State management for restart process const isRestarting = ref(false) @@ -155,7 +155,7 @@ const handleRestart = async () => { await useWorkflowService().reloadCurrentWorkflow() // Run conflict detection after restart completion - await performConflictDetection() + await runFullConflictAnalysis() } finally { await settingStore.set( 'Comfy.Toast.DisableReconnectingToast', diff --git a/src/composables/useConflictDetection.ts b/src/composables/useConflictDetection.ts index 188a71d23..6481fc438 100644 --- a/src/composables/useConflictDetection.ts +++ b/src/composables/useConflictDetection.ts @@ -18,7 +18,7 @@ import type { ConflictDetectionSummary, ConflictType, Node, - NodePackRequirements, + NodeRequirements, SystemEnvironment } from '@/types/conflictDetectionTypes' import { normalizePackId } from '@/utils/packUtils' @@ -75,7 +75,7 @@ export function useConflictDetection() { * Continues with default values even if errors occur. * @returns Promise that resolves to system environment information */ - async function detectSystemEnvironment(): Promise { + async function collectSystemEnvironment(): Promise { try { // Get system stats from store (primary source of system information) // Wait for systemStats to be initialized if not already @@ -88,9 +88,7 @@ export function useConflictDetection() { const environment: SystemEnvironment = { comfyui_version: systemStats?.system.comfyui_version ?? '', frontend_version: frontendVersion, - // Platform information (from system stats) os: systemStats?.system.os ?? '', - // GPU/accelerator information accelerator: systemStats?.devices[0].type ?? '' } @@ -126,7 +124,7 @@ export function useConflictDetection() { * * @returns Promise that resolves to array of node pack requirements */ - async function fetchNodePackRequirements(): Promise { + async function buildNodeRequirements(): Promise { try { // Step 1: Use installed packs composable instead of direct API calls await startFetchInstalled() // Ensure data is loaded @@ -167,7 +165,7 @@ export function useConflictDetection() { abortController.value?.signal ) - if (bulkResponse && bulkResponse.node_versions) { + if (bulkResponse && bulkResponse.node_versions?.length > 0) { // Process bulk response bulkResponse.node_versions.forEach((result) => { if (result.status === 'success' && result.node_version) { @@ -192,30 +190,26 @@ export function useConflictDetection() { } // Step 5: Combine local installation data with Registry version data - const requirements: NodePackRequirements[] = [] + const requirements: NodeRequirements[] = [] // IMPORTANT: Use installedPacksWithVersions to check ALL installed packages // not just the ones that exist in Registry (installedPacks) - for (const installedPack of installedPacksWithVersions.value) { - const packageId = installedPack.id - - // !!!!! 질문: 여기서 버전 데이터가 왜 있음 ??? - const versionData = versionDataMap.get(packageId) - const installedVersion = installedPack.version - - // Check if package is enabled using store method - const isEnabled = managerStore.isPackEnabled(packageId) + for (const installedPackVersion of installedPacksWithVersions.value) { + const versionData = versionDataMap.get(installedPackVersion.id) + const isEnabled = managerStore.isPackEnabled(installedPackVersion.id) // Find the pack info from Registry if available - const packInfo = installedPacks.value.find((p) => p.id === packageId) + const packInfo = installedPacks.value.find( + (p) => p.id === installedPackVersion.id + ) if (versionData) { // Combine local installation data with version-specific Registry data - const requirement: NodePackRequirements = { + const requirement: NodeRequirements = { // Basic package info - id: packageId, - name: packInfo?.name || packageId, - installed_version: installedVersion, + id: installedPackVersion.id, + name: packInfo?.name || installedPackVersion.id, + installed_version: installedPackVersion.version, is_enabled: isEnabled, // Version-specific compatibility data @@ -234,14 +228,14 @@ export function useConflictDetection() { requirements.push(requirement) } else { console.warn( - `[ConflictDetection] No Registry data found for ${packageId}, using fallback` + `[ConflictDetection] No Registry data found for ${installedPackVersion.id}, using fallback` ) // Create fallback requirement without Registry data - const fallbackRequirement: NodePackRequirements = { - id: packageId, - name: packInfo?.name || packageId, - installed_version: installedVersion, + const fallbackRequirement: NodeRequirements = { + id: installedPackVersion.id, + name: packInfo?.name || installedPackVersion.id, + installed_version: installedPackVersion.version, is_enabled: isEnabled, is_banned: false, is_pending: false @@ -268,8 +262,8 @@ export function useConflictDetection() { * @param sysEnv Current system environment * @returns Conflict detection result for the package */ - function detectPackageConflicts( - packageReq: NodePackRequirements, + function analyzePackageConflicts( + packageReq: NodeRequirements, systemEnvInfo: SystemEnvironment ): ConflictDetectionResult { const conflicts: ConflictDetail[] = [] @@ -321,13 +315,13 @@ export function useConflictDetection() { } // 5. Banned package check using shared logic - const bannedConflict = getBannedConflictMessage(packageReq.is_banned) + const bannedConflict = createBannedConflict(packageReq.is_banned) if (bannedConflict) { conflicts.push(bannedConflict) } // 6. Registry data availability check using shared logic - const pendingConflict = getPendingConflictMessage(packageReq.is_pending) + const pendingConflict = createPendingConflict(packageReq.is_pending) if (pendingConflict) { conflicts.push(pendingConflict) } @@ -451,7 +445,7 @@ export function useConflictDetection() { * Performs complete conflict detection. * @returns Promise that resolves to conflict detection response */ - async function performConflictDetection(): Promise { + async function runFullConflictAnalysis(): Promise { if (isDetecting.value) { console.debug('[ConflictDetection] Already detecting, skipping') return { @@ -468,16 +462,16 @@ export function useConflictDetection() { try { // 1. Collect system environment information - const systemEnvInfo = await detectSystemEnvironment() + const systemEnvInfo = await collectSystemEnvironment() - // 2. Collect package requirement information - const packageRequirements = await fetchNodePackRequirements() + // 2. Collect installed node requirement information + const installedNodeRequirements = await buildNodeRequirements() // 3. Detect conflicts for each package (parallel processing) - const conflictDetectionTasks = packageRequirements.map( + const conflictDetectionTasks = installedNodeRequirements.map( async (packageReq) => { try { - return detectPackageConflicts(packageReq, systemEnvInfo) + return analyzePackageConflicts(packageReq, systemEnvInfo) } catch (error) { console.warn( `[ConflictDetection] Failed to detect conflicts for package ${packageReq.name}:`, @@ -573,7 +567,7 @@ export function useConflictDetection() { return { success: false, error_message: detectionError.value, - summary: detectionSummary.value || getEmptySummary(), + summary: detectionSummary.value || generateEmptySummary(), results: [] } } finally { @@ -605,7 +599,7 @@ export function useConflictDetection() { // Manager is new Manager, perform conflict detection // The useInstalledPacks will handle fetching installed list if needed - await performConflictDetection() + await runFullConflictAnalysis() } catch (error) { console.warn( '[ConflictDetection] Error during initialization (ignored):', @@ -647,7 +641,7 @@ export function useConflictDetection() { console.debug( '[ConflictDetection] No detection results, running conflict detection...' ) - await performConflictDetection() + await runFullConflictAnalysis() } // Check if this is a version update scenario @@ -718,7 +712,7 @@ export function useConflictDetection() { } // Check banned package status using shared logic - const bannedConflict = getBannedConflictMessage( + const bannedConflict = createBannedConflict( node.status === 'NodeStatusBanned' || node.status === 'NodeVersionStatusBanned' ) @@ -727,7 +721,7 @@ export function useConflictDetection() { } // Check pending status using shared logic - const pendingConflict = getPendingConflictMessage( + const pendingConflict = createPendingConflict( node.status === 'NodeVersionStatusPending' ) if (pendingConflict) { @@ -756,8 +750,8 @@ export function useConflictDetection() { securityPendingPackages, // Methods - performConflictDetection, - detectSystemEnvironment, + runFullConflictAnalysis, + collectSystemEnvironment, initializeConflictDetection, cancelRequests, shouldShowConflictModalAfterUpdate, @@ -878,7 +872,7 @@ function normalizeOSValues( // TODO: move to type file type OS_TYPE = 'Windows' | 'Linux' | 'MacOS' | 'unknown' -function systemStatsToSupportOs(systemOS?: string): OS_TYPE { +function mapSystemOSToRegistry(systemOS?: string): OS_TYPE { const os = systemOS?.toLowerCase() if (os?.includes('win')) return 'Windows' @@ -893,7 +887,7 @@ function systemStatsToSupportOs(systemOS?: string): OS_TYPE { * @param systemStats System stats data from store * @returns Accelerator information object */ -function extractAcceleratorInfo(systemDeviceType?: string): string { +function mapDeviceTypeToAccelerator(systemDeviceType?: string): string { const deviceType = systemDeviceType?.toLowerCase() switch (deviceType) { @@ -967,7 +961,7 @@ function checkOSConflict( supportedOS: Node['supported_os'], currentOS?: string ): ConflictDetail | null { - const currentOsBySupportOS = systemStatsToSupportOs(currentOS) + const currentOsBySupportOS = mapSystemOSToRegistry(currentOS) const hasOSConflict = currentOS && !supportedOS?.includes(currentOsBySupportOS) if (hasOSConflict) { @@ -989,7 +983,7 @@ function checkAcceleratorConflict( currentAccelerator?: string ): ConflictDetail | null { const currentAcceleratorByAccelerator = - extractAcceleratorInfo(currentAccelerator) + mapDeviceTypeToAccelerator(currentAccelerator) const hasAcceleratorConflict = currentAccelerator && !supportedAccelerators?.includes(currentAcceleratorByAccelerator) @@ -1008,7 +1002,7 @@ function checkAcceleratorConflict( /** * Checks for banned package status conflicts. */ -function getBannedConflictMessage(isBanned?: boolean): ConflictDetail | null { +function createBannedConflict(isBanned?: boolean): ConflictDetail | null { if (isBanned === true) { return { type: 'banned', @@ -1022,7 +1016,7 @@ function getBannedConflictMessage(isBanned?: boolean): ConflictDetail | null { /** * Checks for pending package status conflicts. */ -function getPendingConflictMessage(isPending?: boolean): ConflictDetail | null { +function createPendingConflict(isPending?: boolean): ConflictDetail | null { if (isPending === true) { return { type: 'pending', @@ -1091,7 +1085,7 @@ function generateSummary( /** * Creates an empty summary for error cases. */ -function getEmptySummary(): ConflictDetectionSummary { +function generateEmptySummary(): ConflictDetectionSummary { return { total_packages: 0, compatible_packages: 0, diff --git a/src/types/conflictDetectionTypes.ts b/src/types/conflictDetectionTypes.ts index 50dab5782..f9804fdbd 100644 --- a/src/types/conflictDetectionTypes.ts +++ b/src/types/conflictDetectionTypes.ts @@ -27,7 +27,7 @@ export type ConflictType = * Node Pack requirements from Registry API * Extends Node type with additional installation and compatibility metadata */ -export interface NodePackRequirements extends Node { +export interface NodeRequirements extends Node { installed_version: string is_enabled: boolean is_banned: boolean diff --git a/tests-ui/tests/components/dialog/footer/ManagerProgressFooter.test.ts b/tests-ui/tests/components/dialog/footer/ManagerProgressFooter.test.ts index b63562e81..2e86208b0 100644 --- a/tests-ui/tests/components/dialog/footer/ManagerProgressFooter.test.ts +++ b/tests-ui/tests/components/dialog/footer/ManagerProgressFooter.test.ts @@ -25,7 +25,7 @@ vi.mock('@/services/comfyManagerService') vi.mock('@/composables/useConflictDetection', () => ({ useConflictDetection: vi.fn(() => ({ conflictedPackages: { value: [] }, - performConflictDetection: vi.fn().mockResolvedValue(undefined) + runFullConflictAnalysis: vi.fn().mockResolvedValue(undefined) })) })) diff --git a/tests-ui/tests/composables/useConflictDetection.test.ts b/tests-ui/tests/composables/useConflictDetection.test.ts index 52935f65b..c937cd495 100644 --- a/tests-ui/tests/composables/useConflictDetection.test.ts +++ b/tests-ui/tests/composables/useConflictDetection.test.ts @@ -173,8 +173,8 @@ describe.skip('useConflictDetection with Registry Store', () => { describe('system environment detection', () => { it('should collect system environment information successfully', async () => { - const { detectSystemEnvironment } = useConflictDetection() - const environment = await detectSystemEnvironment() + const { collectSystemEnvironment } = useConflictDetection() + const environment = await collectSystemEnvironment() expect(environment.comfyui_version).toBe('0.3.41') expect(environment.frontend_version).toBe('1.24.0-1') @@ -190,8 +190,8 @@ describe.skip('useConflictDetection with Registry Store', () => { ) mockSystemStatsStore.systemStats = null - const { detectSystemEnvironment } = useConflictDetection() - const environment = await detectSystemEnvironment() + const { collectSystemEnvironment } = useConflictDetection() + const environment = await collectSystemEnvironment() expect(environment.comfyui_version).toBe('unknown') expect(environment.frontend_version).toBe('1.24.0-1') @@ -260,8 +260,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.total_packages).toBeGreaterThanOrEqual(1) @@ -309,8 +309,8 @@ describe.skip('useConflictDetection with Registry Store', () => { // Mock Registry Service returning null (no packages found) mockRegistryService.getPackByVersion.mockResolvedValue(null) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.total_packages).toBe(1) @@ -332,8 +332,8 @@ describe.skip('useConflictDetection with Registry Store', () => { it('should return empty array when local package information cannot be retrieved', async () => { mockComfyManagerService.listInstalledPacks.mockResolvedValue(null) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.total_packages).toBe(0) @@ -377,8 +377,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.conflicted_packages).toBe(0) @@ -420,8 +420,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.conflicted_packages).toBe(1) @@ -472,8 +472,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.conflicted_packages).toBe(1) @@ -523,8 +523,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.banned_packages).toBe(1) @@ -577,8 +577,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.banned_packages).toBe(1) @@ -633,13 +633,13 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { hasConflicts, performConflictDetection } = useConflictDetection() + const { hasConflicts, runFullConflictAnalysis } = useConflictDetection() // Initial value should be false expect(hasConflicts.value).toBe(false) // Execute conflict detection - await performConflictDetection() + await runFullConflictAnalysis() await nextTick() // Should be true when conflicts are detected @@ -680,10 +680,10 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { conflictedPackages, performConflictDetection } = + const { conflictedPackages, runFullConflictAnalysis } = useConflictDetection() - await performConflictDetection() + await runFullConflictAnalysis() await nextTick() expect(conflictedPackages.value.length).toBeGreaterThan(0) @@ -740,10 +740,10 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { bannedPackages, performConflictDetection } = + const { bannedPackages, runFullConflictAnalysis } = useConflictDetection() - await performConflictDetection() + await runFullConflictAnalysis() await nextTick() expect(bannedPackages.value).toHaveLength(1) @@ -766,8 +766,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.detected_system_environment?.comfyui_version).toBe( @@ -781,8 +781,8 @@ describe.skip('useConflictDetection with Registry Store', () => { new Error('Service error') ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.total_packages).toBe(0) @@ -830,8 +830,8 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) expect(result.summary.total_packages).toBeGreaterThanOrEqual(1) @@ -862,8 +862,8 @@ describe.skip('useConflictDetection with Registry Store', () => { new Error('Critical error') ) - const { performConflictDetection } = useConflictDetection() - const result = await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + const result = await runFullConflictAnalysis() expect(result.success).toBe(true) // Error resilience maintains success expect(result.summary.total_packages).toBe(0) @@ -889,8 +889,8 @@ describe.skip('useConflictDetection with Registry Store', () => { status: 'NodeVersionStatusActive' }) - const { performConflictDetection } = useConflictDetection() - await performConflictDetection() + const { runFullConflictAnalysis } = useConflictDetection() + await runFullConflictAnalysis() expect(mockAcknowledgment.checkComfyUIVersionChange).toHaveBeenCalledWith( '0.3.41' @@ -945,11 +945,11 @@ describe.skip('useConflictDetection with Registry Store', () => { } ) - const { shouldShowConflictModalAfterUpdate, performConflictDetection } = + const { shouldShowConflictModalAfterUpdate, runFullConflictAnalysis } = useConflictDetection() // First run conflict detection to populate conflicts - await performConflictDetection() + await runFullConflictAnalysis() await nextTick() // Now check if modal should show after update @@ -967,10 +967,10 @@ describe.skip('useConflictDetection with Registry Store', () => { devices: [] } - const { detectSystemEnvironment } = useConflictDetection() + const { collectSystemEnvironment } = useConflictDetection() // Detect system environment - const environment = await detectSystemEnvironment() + const environment = await collectSystemEnvironment() expect(environment.comfyui_version).toBe('0.3.41') })