From 7489d9d186e68f07deb618e0503f176c19107338 Mon Sep 17 00:00:00 2001 From: Shaun Walker Date: Wed, 9 Nov 2022 21:11:02 -0500 Subject: [PATCH] move UI logic from FileService to FileManager, add progressive retry logic, update file attributes if uploading a new version of a file, clean up temporary artifacts on failure, improve upload efficiency --- .../Modules/Controls/FileManager.razor | 79 ++++++++++------ .../Modules/Controls/FileManager.resx | 2 +- Oqtane.Client/Services/FileService.cs | 60 +----------- .../Services/Interfaces/IFileService.cs | 21 ----- Oqtane.Server/Controllers/FileController.cs | 94 ++++++++++--------- Oqtane.Server/Repository/FileRepository.cs | 17 ++++ .../Repository/Interfaces/IFileRepository.cs | 1 + Oqtane.Shared/Shared/Constants.cs | 2 +- 8 files changed, 122 insertions(+), 154 deletions(-) diff --git a/Oqtane.Client/Modules/Controls/FileManager.razor b/Oqtane.Client/Modules/Controls/FileManager.razor index b189fb74..de6da65f 100644 --- a/Oqtane.Client/Modules/Controls/FileManager.razor +++ b/Oqtane.Client/Modules/Controls/FileManager.razor @@ -1,4 +1,5 @@ @namespace Oqtane.Modules.Controls +@using System.Threading @inherits ModuleControlBase @inject IFolderService FolderService @inject IFileService FileService @@ -53,7 +54,7 @@ }
- + @if (ShowFiles && GetFileId() != -1) { @@ -304,17 +305,17 @@ } } - private async Task UploadFile() + private async Task UploadFiles() { _message = string.Empty; var interop = new Interop(JSRuntime); - var upload = await interop.GetFiles(_fileinputid); - if (upload.Length > 0) + var uploads = await interop.GetFiles(_fileinputid); + if (uploads.Length > 0) { string restricted = ""; - foreach (var file in upload) + foreach (var upload in uploads) { - var extension = (file.LastIndexOf(".") != -1) ? file.Substring(file.LastIndexOf(".") + 1) : ""; + var extension = (upload.LastIndexOf(".") != -1) ? upload.Substring(upload.LastIndexOf(".") + 1) : ""; if (!Constants.UploadableFiles.Split(',').Contains(extension.ToLower())) { restricted += (restricted == "" ? "" : ",") + extension; @@ -324,48 +325,68 @@ { try { - string result; - if (Folder == Constants.PackagesFolder) + // upload the files + var posturl = Utilities.TenantUrl(PageState.Alias, "/api/file/upload"); + var folder = (Folder == Constants.PackagesFolder) ? Folder : FolderId.ToString(); + await interop.UploadFiles(posturl, folder, _guid, SiteState.AntiForgeryToken); + + // uploading is asynchronous so we need to wait for the uploads to complete + // note that this will only wait a maximum of 15 seconds which may not be long enough for very large file uploads + bool success = false; + int attempts = 0; + while (attempts < 5 && !success) { - result = await FileService.UploadFilesAsync(Folder, upload, _guid); - } - else - { - result = await FileService.UploadFilesAsync(FolderId, upload, _guid); + attempts += 1; + Thread.Sleep(1000 * attempts); // progressive retry + + success = true; + List files = await FileService.GetFilesAsync(folder); + if (files.Count > 0) + { + foreach (string upload in uploads) + { + if (!files.Exists(item => item.Name == upload)) + { + success = false; + } + } + } } - if (result == string.Empty) + // reset progress indicators + await interop.SetElementAttribute(_guid + "ProgressInfo", "style", "display: none;"); + await interop.SetElementAttribute(_guid + "ProgressBar", "style", "display: none;"); + + if (success) { - await logger.LogInformation("File Upload Succeeded {Files}", upload); + await logger.LogInformation("File Upload Succeeded {Files}", uploads); if (ShowSuccess) { _message = Localizer["Success.File.Upload"]; _messagetype = MessageType.Success; } - - // set FileId to first file in upload collection - await GetFiles(); - var file = _files.Where(item => item.Name == upload[0]).FirstOrDefault(); - if (file != null) - { - FileId = file.FileId; - await SetImage(); - await OnUpload.InvokeAsync(FileId); - } - StateHasChanged(); } else { - await logger.LogError("File Upload Failed For {Files}", result.Replace(",", ", ")); - + await logger.LogInformation("File Upload Failed Or Is Still In Progress {Files}", uploads); _message = Localizer["Error.File.Upload"]; _messagetype = MessageType.Error; } + + // set FileId to first file in upload collection + await GetFiles(); + var file = _files.Where(item => item.Name == uploads[0]).FirstOrDefault(); + if (file != null) + { + FileId = file.FileId; + await SetImage(); + await OnUpload.InvokeAsync(FileId); + } + StateHasChanged(); } catch (Exception ex) { await logger.LogError(ex, "File Upload Failed {Error}", ex.Message); - _message = Localizer["Error.File.Upload"]; _messagetype = MessageType.Error; } diff --git a/Oqtane.Client/Resources/Modules/Controls/FileManager.resx b/Oqtane.Client/Resources/Modules/Controls/FileManager.resx index c2ccac05..c4e84b7b 100644 --- a/Oqtane.Client/Resources/Modules/Controls/FileManager.resx +++ b/Oqtane.Client/Resources/Modules/Controls/FileManager.resx @@ -127,7 +127,7 @@ Error Loading Files - File Upload Failed + File Upload Failed Or Is Still In Progress You Have Not Selected A File To Upload diff --git a/Oqtane.Client/Services/FileService.cs b/Oqtane.Client/Services/FileService.cs index c01f1160..584e618b 100644 --- a/Oqtane.Client/Services/FileService.cs +++ b/Oqtane.Client/Services/FileService.cs @@ -2,27 +2,17 @@ using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; -using System.Threading; using System.Threading.Tasks; -using Microsoft.JSInterop; using Oqtane.Documentation; using Oqtane.Models; using Oqtane.Shared; -using Oqtane.UI; namespace Oqtane.Services { [PrivateApi("Don't show in the documentation, as everything should use the Interface")] public class FileService : ServiceBase, IFileService { - private readonly SiteState _siteState; - private readonly IJSRuntime _jsRuntime; - - public FileService(HttpClient http, SiteState siteState, IJSRuntime jsRuntime) : base(http, siteState) - { - _siteState = siteState; - _jsRuntime = jsRuntime; - } + public FileService(HttpClient http, SiteState siteState) : base(http, siteState) { } private string Apiurl => CreateApiUrl("File"); @@ -75,54 +65,6 @@ namespace Oqtane.Services return await GetJsonAsync($"{Apiurl}/upload?url={WebUtility.UrlEncode(url)}&folderid={folderId}&name={name}"); } - public async Task UploadFilesAsync(int folderId, string[] files, string id) - { - return await UploadFilesAsync(folderId.ToString(), files, id); - } - - public async Task UploadFilesAsync(string folder, string[] files, string id) - { - string result = ""; - - var interop = new Interop(_jsRuntime); - await interop.UploadFiles($"{Apiurl}/upload", folder, id, _siteState.AntiForgeryToken); - - // uploading files is asynchronous so we need to wait for the upload to complete - bool success = false; - int attempts = 0; - while (attempts < 5 && success == false) - { - Thread.Sleep(2000); // wait 2 seconds - result = ""; - - List fileList = await GetFilesAsync(folder); - if (fileList.Count > 0) - { - success = true; - foreach (string file in files) - { - if (!fileList.Exists(item => item.Name == file)) - { - success = false; - result += file + ","; - } - } - } - - attempts += 1; - } - - await interop.SetElementAttribute(id + "ProgressInfo", "style", "display: none;"); - await interop.SetElementAttribute(id + "ProgressBar", "style", "display: none;"); - - if (!success) - { - result = result.Substring(0, result.Length - 1); - } - - return result; - } - public async Task DownloadFileAsync(int fileId) { return await GetByteArrayAsync($"{Apiurl}/download/{fileId}"); diff --git a/Oqtane.Client/Services/Interfaces/IFileService.cs b/Oqtane.Client/Services/Interfaces/IFileService.cs index f46d8fca..66553b0c 100644 --- a/Oqtane.Client/Services/Interfaces/IFileService.cs +++ b/Oqtane.Client/Services/Interfaces/IFileService.cs @@ -66,27 +66,6 @@ namespace Oqtane.Services /// Task UploadFileAsync(string url, int folderId, string name); - /// - /// Upload one or more files. - /// - /// Target - /// The files to upload, serialized as a string. - /// A task-identifier, to ensure communication about this upload. - /// - Task UploadFilesAsync(int folderId, string[] files, string fileUploadName); - - - /// - /// Upload one or more files. - /// - /// Target - /// TODO: todoc verify exactly from where the folder path must start - /// - /// The files to upload, serialized as a string. - /// A task-identifier, to ensure communication about this upload. - /// - Task UploadFilesAsync(string folder, string[] files, string fileUploadName); - /// /// Get / download a file (the body). /// diff --git a/Oqtane.Server/Controllers/FileController.cs b/Oqtane.Server/Controllers/FileController.cs index 26386be3..cd6a9087 100644 --- a/Oqtane.Server/Controllers/FileController.cs +++ b/Oqtane.Server/Controllers/FileController.cs @@ -339,7 +339,15 @@ namespace Oqtane.Controllers var file = CreateFile(upload, FolderId, Path.Combine(folderPath, upload)); if (file != null) { - file = _files.AddFile(file); + if (file.FileId == 0) + { + file = _files.AddFile(file); + } + else + { + file = _files.UpdateFile(file); + } + _logger.Log(LogLevel.Information, this, LogFunction.Create, "File Upload Succeeded {File}", Path.Combine(folderPath, upload)); _syncManager.AddSyncEvent(_alias.TenantId, EntityNames.File, file.FileId, SyncEventActions.Create); } } @@ -361,16 +369,16 @@ namespace Oqtane.Controllers int totalparts = int.Parse(parts?.Substring(parts.IndexOf("_") + 1)); filename = Path.GetFileNameWithoutExtension(filename); // base filename - string[] fileParts = Directory.GetFiles(folder, filename + token + "*"); // list of all file parts + string[] fileparts = Directory.GetFiles(folder, filename + token + "*"); // list of all file parts // if all of the file parts exist ( note that file parts can arrive out of order ) - if (fileParts.Length == totalparts && CanAccessFiles(fileParts)) + if (fileparts.Length == totalparts && CanAccessFiles(fileparts)) { - // merge file parts + // merge file parts into temp file ( in case another user is trying to get the file ) bool success = true; using (var stream = new FileStream(Path.Combine(folder, filename + ".tmp"), FileMode.Create)) { - foreach (string filepart in fileParts) + foreach (string filepart in fileparts) { try { @@ -386,52 +394,49 @@ namespace Oqtane.Controllers } } - // delete file parts and rename file + // clean up file parts + foreach (var file in Directory.GetFiles(folder, "*" + token + "*")) + { + // file name matches part or is more than 2 hours old (ie. a prior file upload failed) + if (fileparts.Contains(file) || System.IO.File.GetCreationTime(file).ToUniversalTime() < DateTime.UtcNow.AddHours(-2)) + { + System.IO.File.Delete(file); + } + } + + // rename temp file if (success) { - foreach (string filepart in fileParts) + // remove file if it already exists (as well as any thumbnails) + foreach (var file in Directory.GetFiles(folder, Path.GetFileNameWithoutExtension(filename) + ".*")) { - System.IO.File.Delete(filepart); + if (Path.GetExtension(file) != ".tmp") + { + System.IO.File.Delete(file); + } } - // remove file if it already exists - if (System.IO.File.Exists(Path.Combine(folder, filename))) - { - System.IO.File.Delete(Path.Combine(folder, filename)); - } - - // rename file now that the entire process is completed + // rename temp file now that the entire process is completed System.IO.File.Move(Path.Combine(folder, filename + ".tmp"), Path.Combine(folder, filename)); - _logger.Log(LogLevel.Information, this, LogFunction.Create, "File Uploaded {File}", Path.Combine(folder, filename)); + // return filename merged = filename; } } - // clean up file parts which are more than 2 hours old ( which can happen if a prior file upload failed ) - var cleanupFiles = Directory.EnumerateFiles(folder, "*" + token + "*") - .Where(f => Path.GetExtension(f).StartsWith(token) && !Path.GetFileName(f).StartsWith(filename)); - foreach (var file in cleanupFiles) - { - var createdDate = System.IO.File.GetCreationTime(file).ToUniversalTime(); - if (createdDate < DateTime.UtcNow.AddHours(-2)) - { - System.IO.File.Delete(file); - } - } - return merged; } private bool CanAccessFiles(string[] files) { - // ensure files are not locked by another process ( ie. still being written to ) - bool canaccess = true; + // ensure files are not locked by another process FileStream stream = null; + bool locked = false; foreach (string file in files) { + locked = true; int attempts = 0; - bool locked = true; + // note that this will wait a maximum of 15 seconds for a file to become unlocked while (attempts < 5 && locked) { try @@ -441,7 +446,8 @@ namespace Oqtane.Controllers } catch // file is locked by another process { - Thread.Sleep(1000); // wait 1 second + attempts += 1; + Thread.Sleep(1000 * attempts); // progressive retry } finally { @@ -450,20 +456,15 @@ namespace Oqtane.Controllers stream.Close(); } } - - attempts += 1; } - - if (locked && canaccess) + if (locked) { - canaccess = false; + break; // file still locked after retrying } } - - return canaccess; + return !locked; } - /// /// Get file with header /// Content-Disposition: inline @@ -655,7 +656,7 @@ namespace Oqtane.Controllers private Models.File CreateFile(string filename, int folderid, string filepath) { - Models.File file = null; + var file = _files.GetFile(folderid, filename); int size = 0; var folder = _folders.GetFolder(folderid); @@ -670,7 +671,10 @@ namespace Oqtane.Controllers FileInfo fileinfo = new FileInfo(filepath); if (folder.Capacity == 0 || ((size + fileinfo.Length) / 1000000) < folder.Capacity) { - file = new Models.File(); + if (file == null) + { + file = new Models.File(); + } file.Name = filename; file.FolderId = folderid; @@ -700,7 +704,11 @@ namespace Oqtane.Controllers else { System.IO.File.Delete(filepath); - _logger.Log(LogLevel.Warning, this, LogFunction.Create, "File Exceeds Folder Capacity {Folder} {File}", folder, filepath); + if (file != null) + { + _files.DeleteFile(file.FileId); + } + _logger.Log(LogLevel.Warning, this, LogFunction.Create, "File Exceeds Folder Capacity And Has Been Removed {Folder} {File}", folder, filepath); } return file; diff --git a/Oqtane.Server/Repository/FileRepository.cs b/Oqtane.Server/Repository/FileRepository.cs index 599bd4bd..bd76e18f 100644 --- a/Oqtane.Server/Repository/FileRepository.cs +++ b/Oqtane.Server/Repository/FileRepository.cs @@ -83,6 +83,23 @@ namespace Oqtane.Repository return file; } + public File GetFile(int folderId, string fileName) + { + var file = _db.File.AsNoTracking() + .Include(item => item.Folder) + .FirstOrDefault(item => item.FolderId == folderId && + item.Name.ToLower() == fileName); + + if (file != null) + { + IEnumerable permissions = _permissions.GetPermissions(file.Folder.SiteId, EntityNames.Folder, file.FolderId).ToList(); + file.Folder.Permissions = permissions.EncodePermissions(); + file.Url = GetFileUrl(file, _tenants.GetAlias()); + } + + return file; + } + public File GetFile(int siteId, string folderPath, string fileName) { var file = _db.File.AsNoTracking() diff --git a/Oqtane.Server/Repository/Interfaces/IFileRepository.cs b/Oqtane.Server/Repository/Interfaces/IFileRepository.cs index 0da50f09..556d02de 100644 --- a/Oqtane.Server/Repository/Interfaces/IFileRepository.cs +++ b/Oqtane.Server/Repository/Interfaces/IFileRepository.cs @@ -10,6 +10,7 @@ namespace Oqtane.Repository File UpdateFile(File file); File GetFile(int fileId); File GetFile(int fileId, bool tracking); + File GetFile(int folderId, string fileName); File GetFile(int siteId, string folderPath, string fileName); void DeleteFile(int fileId); string GetFilePath(int fileId); diff --git a/Oqtane.Shared/Shared/Constants.cs b/Oqtane.Shared/Shared/Constants.cs index 1efceed1..2d1a7980 100644 --- a/Oqtane.Shared/Shared/Constants.cs +++ b/Oqtane.Shared/Shared/Constants.cs @@ -46,7 +46,7 @@ namespace Oqtane.Shared public const string DefaultSite = "Default Site"; public const string ImageFiles = "jpg,jpeg,jpe,gif,bmp,png,ico,webp"; - public const string UploadableFiles = ImageFiles + ",mov,wmv,avi,mp4,mp3,doc,docx,xls,xlsx,ppt,pptx,pdf,txt,zip,nupkg,csv,json,xml,xslt,rss,html,htm,css"; + public const string UploadableFiles = ImageFiles + ",mov,wmv,avi,mp4,mp3,doc,docx,xls,xlsx,ppt,pptx,pdf,txt,zip,nupkg,csv,json,xml,rss,css"; public const string ReservedDevices = "CON,NUL,PRN,COM0,COM1,COM2,COM3,COM4,COM5,COM6,COM7,COM8,COM9,LPT0,LPT1,LPT2,LPT3,LPT4,LPT5,LPT6,LPT7,LPT8,LPT9,CONIN$,CONOUT$"; public static readonly char[] InvalidFileNameChars =