From dfa5829cbd6a0c844e271636ffed0d89ddedf18a Mon Sep 17 00:00:00 2001 From: Robert McRackan Date: Tue, 12 Oct 2021 17:05:01 -0400 Subject: [PATCH] Safe(r)Delete, Safe(r)Move : could have infinite loop of exceptions. Fixed. Limit 3 --- AaxDecrypter/AaxcDownloadConverter.cs | 4 +- AaxDecrypter/AudiobookDownloadBase.cs | 10 +-- AppScaffolding/AppScaffolding.csproj | 2 +- FileManager/FileManager.csproj | 1 + FileManager/FileUtility.cs | 90 +++++++++---------- .../LibationFileManager.csproj | 5 -- 6 files changed, 50 insertions(+), 62 deletions(-) diff --git a/AaxDecrypter/AaxcDownloadConverter.cs b/AaxDecrypter/AaxcDownloadConverter.cs index 9cac8027..e2f1081d 100644 --- a/AaxDecrypter/AaxcDownloadConverter.cs +++ b/AaxDecrypter/AaxcDownloadConverter.cs @@ -65,7 +65,7 @@ namespace AaxDecrypter { var zeroProgress = Step2_Start(); - FileUtility.SafeDelete(OutputFileName); + FileUtility.SaferDelete(OutputFileName); var outputFile = File.Open(OutputFileName, FileMode.OpenOrCreate, FileAccess.ReadWrite); @@ -202,7 +202,7 @@ That naming may not be desirable for everyone, but it's an easy change to instea var fileName = FileUtility.GetMultipartFileName(OutputFileName, currentChapter, splitChapters.Count, newSplitCallback.Chapter.Title); multiPartFilePaths.Add(fileName); - FileUtility.SafeDelete(fileName); + FileUtility.SaferDelete(fileName); newSplitCallback.OutputFile = File.Open(fileName, FileMode.OpenOrCreate); } diff --git a/AaxDecrypter/AudiobookDownloadBase.cs b/AaxDecrypter/AudiobookDownloadBase.cs index 270997fe..3c2e638b 100644 --- a/AaxDecrypter/AudiobookDownloadBase.cs +++ b/AaxDecrypter/AudiobookDownloadBase.cs @@ -47,7 +47,7 @@ namespace AaxDecrypter CacheDir = cacheDirectory; // delete file after validation is complete - FileUtility.SafeDelete(OutputFileName); + FileUtility.SaferDelete(OutputFileName); DownloadLicense = ArgumentValidator.EnsureNotNull(dlLic, nameof(dlLic)); } @@ -116,8 +116,8 @@ namespace AaxDecrypter protected bool Step4_Cleanup() { - FileUtility.SafeDelete(jsonDownloadState); - FileUtility.SafeDelete(tempFile); + FileUtility.SaferDelete(jsonDownloadState); + FileUtility.SaferDelete(tempFile); return !IsCanceled; } @@ -136,8 +136,8 @@ namespace AaxDecrypter } catch { - FileUtility.SafeDelete(jsonDownloadState); - FileUtility.SafeDelete(tempFile); + FileUtility.SaferDelete(jsonDownloadState); + FileUtility.SaferDelete(tempFile); return NewNetworkFilePersister(); } } diff --git a/AppScaffolding/AppScaffolding.csproj b/AppScaffolding/AppScaffolding.csproj index a9612435..981b7c52 100644 --- a/AppScaffolding/AppScaffolding.csproj +++ b/AppScaffolding/AppScaffolding.csproj @@ -3,7 +3,7 @@ net5.0 - 6.2.6.7 + 6.2.6.8 diff --git a/FileManager/FileManager.csproj b/FileManager/FileManager.csproj index c908f72c..eda85125 100644 --- a/FileManager/FileManager.csproj +++ b/FileManager/FileManager.csproj @@ -6,6 +6,7 @@ + diff --git a/FileManager/FileUtility.cs b/FileManager/FileUtility.cs index a6122aa9..9bfcb3fb 100644 --- a/FileManager/FileUtility.cs +++ b/FileManager/FileUtility.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.IO; using System.Linq; using Dinah.Core; +using Polly; +using Polly.Retry; namespace FileManager { @@ -11,8 +13,7 @@ namespace FileManager private const int MAX_FILENAME_LENGTH = 255; private const int MAX_DIRECTORY_LENGTH = 247; -//public static string GetValidFilename(string template, Dictionary parameters) { } - public static string GetValidFilename(string dirFullPath, string filename, string extension, params string[] metadataSuffixes) + public static string GetValidFilename(string dirFullPath, string filename, string extension, string metadataSuffix) { if (string.IsNullOrWhiteSpace(dirFullPath)) throw new ArgumentException($"{nameof(dirFullPath)} may not be null or whitespace", nameof(dirFullPath)); @@ -23,13 +24,11 @@ namespace FileManager filename = filename.Replace(':', '_'); filename = PathLib.ToPathSafeString(filename); - // manage length if (filename.Length > 50) filename = filename.Substring(0, 50) + "[...]"; - // append metadata - if (metadataSuffixes != null && metadataSuffixes.Length > 0) - filename += " [" + string.Join("][", metadataSuffixes) + "]"; + if (!string.IsNullOrWhiteSpace(metadataSuffix)) + filename += $" [{metadataSuffix}]"; // extension is null when this method is used for directory names if (!string.IsNullOrWhiteSpace(extension)) @@ -65,7 +64,7 @@ namespace FileManager public static string Move(string source, string destination) { // TODO: destination must be valid path. Use: " (#)" when needed - SafeMove(source, destination); + SaferMove(source, destination); return destination; } @@ -75,57 +74,50 @@ namespace FileManager return path; } - /// Delete file. No error when file does not exist. - /// Exceptions are logged, not thrown. - /// File to delete - public static void SafeDelete(string source) - { - if (!File.Exists(source)) - return; + private static int maxRetryAttempts { get; } = 3; + private static TimeSpan pauseBetweenFailures { get; } = TimeSpan.FromMilliseconds(100); + private static RetryPolicy retryPolicy { get; } = + Policy + .Handle() + .WaitAndRetry(maxRetryAttempts, i => pauseBetweenFailures); - while (true) + /// Delete file. No error when source does not exist. Retry up to 3 times. + public static void SaferDelete(string source) + => retryPolicy.Execute(() => { try { - File.Delete(source); - Serilog.Log.Logger.Information($"File successfully deleted: {source}"); - break; - } - catch (Exception e) - { - System.Threading.Thread.Sleep(100); - Serilog.Log.Logger.Error(e, $"Failed to delete: {source}"); - } - } - } - - /// - /// Moves a specified file to a new location, providing the option to specify a newfile name. - /// Exceptions are logged, not thrown. - /// - /// The name of the file to move. Can include a relative or absolute path. - /// The new path and name for the file. - public static void SafeMove(string source, string target) - { - while (true) - { - try - { - if (File.Exists(source)) + if (!File.Exists(source)) { - SafeDelete(target); - File.Move(source, target); - Serilog.Log.Logger.Information($"File successfully moved from '{source}' to '{target}'"); + File.Delete(source); + Serilog.Log.Logger.Information("File successfully deleted", new { source }); } - - break; } catch (Exception e) { - System.Threading.Thread.Sleep(100); - Serilog.Log.Logger.Error(e, $"Failed to move '{source}' to '{target}'"); + Serilog.Log.Logger.Error(e, "Failed to delete file", new { source }); + throw; } - } - } + }); + + /// Move file. No error when source does not exist. Retry up to 3 times. + public static void SaferMove(string source, string target) + => retryPolicy.Execute(() => + { + try + { + if (!File.Exists(source)) + { + SaferDelete(target); + File.Move(source, target); + Serilog.Log.Logger.Information("File successfully moved", new { source, target }); + } + } + catch (Exception e) + { + Serilog.Log.Logger.Error(e, "Failed to move file", new { source, target }); + throw; + } + }); } } diff --git a/LibationFileManager/LibationFileManager.csproj b/LibationFileManager/LibationFileManager.csproj index af133cc4..9c5e806b 100644 --- a/LibationFileManager/LibationFileManager.csproj +++ b/LibationFileManager/LibationFileManager.csproj @@ -4,11 +4,6 @@ net5.0 - - - - -