From 919fed05c5c42344540dcf013da59e0abc0cd222 Mon Sep 17 00:00:00 2001 From: Will Miao <13051207myq@gmail.com> Date: Mon, 25 Aug 2025 13:08:35 +0800 Subject: [PATCH] feat: Enhance model moving functionality with improved error handling and unique filename generation --- py/routes/base_model_routes.py | 60 ++++++++++++++++++------------- py/services/model_scanner.py | 59 ++++++++++++++++++++++++------ py/utils/models.py | 38 ++++++++++++++++++++ py/utils/routes_common.py | 1 + static/js/api/baseModelApi.js | 17 +++++---- static/js/managers/MoveManager.js | 44 +++++++++++++---------- 6 files changed, 159 insertions(+), 60 deletions(-) diff --git a/py/routes/base_model_routes.py b/py/routes/base_model_routes.py index 9049d697..affed6fb 100644 --- a/py/routes/base_model_routes.py +++ b/py/routes/base_model_routes.py @@ -684,19 +684,27 @@ class BaseModelRoutes(ABC): source_dir = os.path.dirname(file_path) if os.path.normpath(source_dir) == os.path.normpath(target_path): logger.info(f"Source and target directories are the same: {source_dir}") - return web.json_response({'success': True, 'message': 'Source and target directories are the same'}) - file_name = os.path.basename(file_path) - target_file_path = os.path.join(target_path, file_name).replace(os.sep, '/') - if os.path.exists(target_file_path): return web.json_response({ - 'success': False, - 'error': f"Target file already exists: {target_file_path}" - }, status=409) - success = await self.service.scanner.move_model(file_path, target_path) - if success: - return web.json_response({'success': True, 'new_file_path': target_file_path}) + 'success': True, + 'message': 'Source and target directories are the same', + 'original_file_path': file_path, + 'new_file_path': file_path + }) + + new_file_path = await self.service.scanner.move_model(file_path, target_path) + if new_file_path: + return web.json_response({ + 'success': True, + 'original_file_path': file_path, + 'new_file_path': new_file_path + }) else: - return web.Response(text='Failed to move model', status=500) + return web.json_response({ + 'success': False, + 'error': 'Failed to move model', + 'original_file_path': file_path, + 'new_file_path': None + }, status=500) except Exception as e: logger.error(f"Error moving model: {e}", exc_info=True) return web.Response(text=str(e), status=500) @@ -715,26 +723,28 @@ class BaseModelRoutes(ABC): source_dir = os.path.dirname(file_path) if os.path.normpath(source_dir) == os.path.normpath(target_path): results.append({ - "path": file_path, + "original_file_path": file_path, + "new_file_path": file_path, "success": True, "message": "Source and target directories are the same" }) continue - file_name = os.path.basename(file_path) - target_file_path = os.path.join(target_path, file_name).replace(os.sep, '/') - if os.path.exists(target_file_path): + + new_file_path = await self.service.scanner.move_model(file_path, target_path) + if new_file_path: results.append({ - "path": file_path, - "success": False, - "message": f"Target file already exists: {target_file_path}" + "original_file_path": file_path, + "new_file_path": new_file_path, + "success": True, + "message": "Success" + }) + else: + results.append({ + "original_file_path": file_path, + "new_file_path": None, + "success": False, + "message": "Failed to move model" }) - continue - success = await self.service.scanner.move_model(file_path, target_path) - results.append({ - "path": file_path, - "success": success, - "message": "Success" if success else "Failed to move model" - }) success_count = sum(1 for r in results if r["success"]) failure_count = len(results) - success_count return web.json_response({ diff --git a/py/services/model_scanner.py b/py/services/model_scanner.py index 9eef1ca6..523d18c1 100644 --- a/py/services/model_scanner.py +++ b/py/services/model_scanner.py @@ -792,8 +792,16 @@ class ModelScanner: logger.error(f"Error adding model to cache: {e}") return False - async def move_model(self, source_path: str, target_path: str) -> bool: - """Move a model and its associated files to a new location""" + async def move_model(self, source_path: str, target_path: str) -> Optional[str]: + """Move a model and its associated files to a new location + + Args: + source_path: Original file path + target_path: Target directory path + + Returns: + Optional[str]: New file path if successful, None if failed + """ try: source_path = source_path.replace(os.sep, '/') target_path = target_path.replace(os.sep, '/') @@ -802,14 +810,37 @@ class ModelScanner: if not file_ext or file_ext.lower() not in self.file_extensions: logger.error(f"Invalid file extension for model: {file_ext}") - return False + return None base_name = os.path.splitext(os.path.basename(source_path))[0] source_dir = os.path.dirname(source_path) os.makedirs(target_path, exist_ok=True) - target_file = os.path.join(target_path, f"{base_name}{file_ext}").replace(os.sep, '/') + # Get SHA256 hash of the source file for conflict resolution + source_hash = self.get_hash_by_path(source_path) + if not source_hash: + # Calculate hash if not in cache + try: + import hashlib + with open(source_path, 'rb') as f: + source_hash = hashlib.sha256(f.read()).hexdigest().lower() + except Exception as e: + logger.error(f"Failed to calculate hash for {source_path}: {e}") + source_hash = "unknown" + + # Check for filename conflicts and auto-rename if necessary + from ..utils.models import BaseModelMetadata + final_filename = BaseModelMetadata.generate_unique_filename( + target_path, base_name, file_ext, source_hash + ) + + target_file = os.path.join(target_path, final_filename).replace(os.sep, '/') + final_base_name = os.path.splitext(final_filename)[0] + + # Log if filename was changed due to conflict + if final_filename != f"{base_name}{file_ext}": + logger.info(f"Renamed {base_name}{file_ext} to {final_filename} to avoid filename conflict") real_source = os.path.realpath(source_path) real_target = os.path.realpath(target_file) @@ -826,12 +857,17 @@ class ModelScanner: for file in os.listdir(source_dir): if file.startswith(base_name + ".") and file != os.path.basename(source_path): source_file_path = os.path.join(source_dir, file) + # Generate new filename with the same base name as the model file + file_suffix = file[len(base_name):] # Get the part after base_name (e.g., ".metadata.json", ".preview.png") + new_associated_filename = f"{final_base_name}{file_suffix}" + target_associated_path = os.path.join(target_path, new_associated_filename) + # Store metadata file path for special handling if file == f"{base_name}.metadata.json": source_metadata = source_file_path - moved_metadata_path = os.path.join(target_path, file) + moved_metadata_path = target_associated_path else: - files_to_move.append((source_file_path, os.path.join(target_path, file))) + files_to_move.append((source_file_path, target_associated_path)) except Exception as e: logger.error(f"Error listing files in {source_dir}: {e}") @@ -853,11 +889,11 @@ class ModelScanner: await self.update_single_model_cache(source_path, target_file, metadata) - return True + return target_file except Exception as e: logger.error(f"Error moving model: {e}", exc_info=True) - return False + return None async def _update_metadata_paths(self, metadata_path: str, model_path: str) -> Dict: """Update file paths in metadata file""" @@ -866,12 +902,15 @@ class ModelScanner: metadata = json.load(f) metadata['file_path'] = model_path.replace(os.sep, '/') + # Update file_name to match the new filename + metadata['file_name'] = os.path.splitext(os.path.basename(model_path))[0] if 'preview_url' in metadata and metadata['preview_url']: preview_dir = os.path.dirname(model_path) - preview_name = os.path.splitext(os.path.basename(metadata['preview_url']))[0] + # Update preview filename to match the new base name + new_base_name = os.path.splitext(os.path.basename(model_path))[0] preview_ext = os.path.splitext(metadata['preview_url'])[1] - new_preview_path = os.path.join(preview_dir, f"{preview_name}{preview_ext}") + new_preview_path = os.path.join(preview_dir, f"{new_base_name}{preview_ext}") metadata['preview_url'] = new_preview_path.replace(os.sep, '/') await MetadataManager.save_metadata(metadata_path, metadata) diff --git a/py/utils/models.py b/py/utils/models.py index ac2650d7..6b13f013 100644 --- a/py/utils/models.py +++ b/py/utils/models.py @@ -83,6 +83,44 @@ class BaseModelMetadata: self.size = os.path.getsize(file_path) self.modified = os.path.getmtime(file_path) self.file_path = file_path.replace(os.sep, '/') + # Update file_name when file_path changes + self.file_name = os.path.splitext(os.path.basename(file_path))[0] + + @staticmethod + def generate_unique_filename(target_dir: str, base_name: str, extension: str, sha256_hash: str) -> str: + """Generate a unique filename to avoid conflicts + + Args: + target_dir: Target directory path + base_name: Base filename without extension + extension: File extension including the dot + sha256_hash: SHA256 hash of the file for generating short hash + + Returns: + str: Unique filename that doesn't conflict with existing files + """ + original_filename = f"{base_name}{extension}" + target_path = os.path.join(target_dir, original_filename) + + # If no conflict, return original filename + if not os.path.exists(target_path): + return original_filename + + # Generate short hash (first 4 characters of SHA256) + short_hash = sha256_hash[:4] if sha256_hash else "0000" + + # Try with short hash suffix + unique_filename = f"{base_name}-{short_hash}{extension}" + unique_path = os.path.join(target_dir, unique_filename) + + # If still conflicts, add incremental number + counter = 1 + while os.path.exists(unique_path): + unique_filename = f"{base_name}-{short_hash}-{counter}{extension}" + unique_path = os.path.join(target_dir, unique_filename) + counter += 1 + + return unique_filename @dataclass class LoraMetadata(BaseModelMetadata): diff --git a/py/utils/routes_common.py b/py/utils/routes_common.py index 1f32b369..47554903 100644 --- a/py/utils/routes_common.py +++ b/py/utils/routes_common.py @@ -982,6 +982,7 @@ class ModelRouteUtils: # Rename all files renamed_files = [] new_metadata_path = None + new_preview = None for old_path, pattern in existing_files: # Get the file extension like .safetensors or .metadata.json diff --git a/static/js/api/baseModelApi.js b/static/js/api/baseModelApi.js index 6b58145c..169ca902 100644 --- a/static/js/api/baseModelApi.js +++ b/static/js/api/baseModelApi.js @@ -419,6 +419,7 @@ export class BaseModelApiClient { }; }); + // Wait for WebSocket connection to establish await new Promise((resolve, reject) => { ws.onopen = resolve; ws.onerror = reject; @@ -434,6 +435,7 @@ export class BaseModelApiClient { throw new Error('Failed to fetch metadata'); } + // Wait for the operation to complete via WebSocket await operationComplete; resetAndReload(false); @@ -749,7 +751,10 @@ export class BaseModelApiClient { } if (result.success) { - return result.new_file_path; + return { + original_file_path: result.original_file_path || filePath, + new_file_path: result.new_file_path + }; } return null; } @@ -785,7 +790,6 @@ export class BaseModelApiClient { throw new Error(`Failed to move ${this.apiConfig.config.displayName}s`); } - let successFilePaths = []; if (result.success) { if (result.failure_count > 0) { showToast(`Moved ${result.success_count} ${this.apiConfig.config.displayName}s, ${result.failure_count} failed`, 'warning'); @@ -793,7 +797,7 @@ export class BaseModelApiClient { const failedFiles = result.results .filter(r => !r.success) .map(r => { - const fileName = r.path.substring(r.path.lastIndexOf('/') + 1); + const fileName = r.original_file_path.substring(r.original_file_path.lastIndexOf('/') + 1); return `${fileName}: ${r.message}`; }); if (failedFiles.length > 0) { @@ -805,13 +809,12 @@ export class BaseModelApiClient { } else { showToast(`Successfully moved ${result.success_count} ${this.apiConfig.config.displayName}s`, 'success'); } - successFilePaths = result.results - .filter(r => r.success) - .map(r => r.path); + + // Return the results array with original_file_path and new_file_path + return result.results || []; } else { throw new Error(result.message || `Failed to move ${this.apiConfig.config.displayName}s`); } - return successFilePaths; } async bulkDeleteModels(filePaths) { diff --git a/static/js/managers/MoveManager.js b/static/js/managers/MoveManager.js index ab17b2e6..fac44040 100644 --- a/static/js/managers/MoveManager.js +++ b/static/js/managers/MoveManager.js @@ -177,40 +177,48 @@ class MoveManager { try { if (this.bulkFilePaths) { // Bulk move mode - const movedFilePaths = await apiClient.moveBulkModels(this.bulkFilePaths, targetPath); + const results = await apiClient.moveBulkModels(this.bulkFilePaths, targetPath); // Update virtual scroller if in active folder view const pageState = getCurrentPageState(); if (pageState.activeFolder !== null && state.virtualScroller) { - // Remove only successfully moved items - movedFilePaths.forEach(newFilePath => { - // Find original filePath by matching filename - const filename = newFilePath.substring(newFilePath.lastIndexOf('/') + 1); - const originalFilePath = this.bulkFilePaths.find(fp => fp.endsWith('/' + filename)); - if (originalFilePath) { - state.virtualScroller.removeItemByFilePath(originalFilePath); + // Remove items that were successfully moved + results.forEach(result => { + if (result.success) { + state.virtualScroller.removeItemByFilePath(result.original_file_path); } }); } else { - // Update the model cards' filepath in the DOM - movedFilePaths.forEach(newFilePath => { - const filename = newFilePath.substring(newFilePath.lastIndexOf('/') + 1); - const originalFilePath = this.bulkFilePaths.find(fp => fp.endsWith('/' + filename)); - if (originalFilePath) { - state.virtualScroller.updateSingleItem(originalFilePath, {file_path: newFilePath}); + // Update the model cards' filepath and filename in the DOM + results.forEach(result => { + if (result.success && result.new_file_path !== result.original_file_path) { + const newFileName = result.new_file_path.substring(result.new_file_path.lastIndexOf('/') + 1); + const baseFileName = newFileName.substring(0, newFileName.lastIndexOf('.')); + + state.virtualScroller.updateSingleItem(result.original_file_path, { + file_path: result.new_file_path, + file_name: baseFileName + }); } }); } } else { // Single move mode - const newFilePath = await apiClient.moveSingleModel(this.currentFilePath, targetPath); + const result = await apiClient.moveSingleModel(this.currentFilePath, targetPath); const pageState = getCurrentPageState(); - if (newFilePath) { + if (result && result.new_file_path) { if (pageState.activeFolder !== null && state.virtualScroller) { state.virtualScroller.removeItemByFilePath(this.currentFilePath); - } else { - state.virtualScroller.updateSingleItem(this.currentFilePath, {file_path: newFilePath}); + } else if (result.new_file_path !== this.currentFilePath) { + // Update both file_path and file_name if they changed + const newFileName = result.new_file_path.substring(result.new_file_path.lastIndexOf('/') + 1); + const baseFileName = newFileName.substring(0, newFileName.lastIndexOf('.')); + + state.virtualScroller.updateSingleItem(this.currentFilePath, { + file_path: result.new_file_path, + file_name: baseFileName + }); } } }