From 2a3c54d3bab5bf7e01fe917e7d2ed12b00bd4b6c Mon Sep 17 00:00:00 2001 From: Friedrich von Never Date: Sun, 28 Aug 2022 15:20:14 +0200 Subject: [PATCH] (#102) FileCache: improve the workarounds for the older versions of Windows --- Emulsion.ContentProxy/FileCache.fs | 65 +++++++++++-------- ...0220828132228_UpdatedUniqueContentIndex.fs | 63 ++++++++++++++++++ Emulsion.Tests/ContentProxy/FileCacheTests.fs | 4 ++ 3 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 Emulsion.Database/Migrations/20220828132228_UpdatedUniqueContentIndex.fs diff --git a/Emulsion.ContentProxy/FileCache.fs b/Emulsion.ContentProxy/FileCache.fs index 2339aeba..73a303e7 100644 --- a/Emulsion.ContentProxy/FileCache.fs +++ b/Emulsion.ContentProxy/FileCache.fs @@ -36,6 +36,26 @@ module FileCache = with | :? ArgumentException -> None + let IsMoveAndDeleteModeEnabled = + // NOTE: On older versions of Windows (known to reproduce on windows-2019 GitHub Actions image), the following + // scenario may be defunct: + // + // - open a file with FileShare.Delete (i.e. for download) + // - delete a file (i.e. during the cache cleanup) + // - try to create a file with the same name again + // + // According to this article + // (https://boostgsoc13.github.io/boost.afio/doc/html/afio/FAQ/deleting_open_files.html), it is impossible to do + // since file will occupy its disk name until the last handle is closed. + // + // In practice, this is allowed (checked at least on Windows 10 20H2 and windows-2022 GitHub Actions image), but + // some tests are known to be broken on older versions of Windows (windows-2019). + // + // As a workaround, let's rename the file to a random name before deleting it. + // + // This workaround may be removed after these older versions of Windows goes out of support. + OperatingSystem.IsWindows() + type FileCache(logger: ILogger, settings: FileCacheSettings, httpClientFactory: IHttpClientFactory, @@ -61,12 +81,27 @@ type FileCache(logger: ILogger, None } + let enumerateCacheFiles() = + let entries = Directory.EnumerateFileSystemEntries settings.Directory + if FileCache.IsMoveAndDeleteModeEnabled then + entries |> Seq.filter(fun p -> not(p.EndsWith ".deleted")) + else + entries + + let deleteFileSafe (fileInfo: FileInfo) = async { + if FileCache.IsMoveAndDeleteModeEnabled then + fileInfo.MoveTo(Path.Combine(fileInfo.DirectoryName, $"{Guid.NewGuid().ToString()}.deleted")) + fileInfo.Delete() + else + fileInfo.Delete() + } + let assertCacheDirectoryExists() = async { Directory.CreateDirectory settings.Directory |> ignore } let assertCacheValid() = async { - Directory.EnumerateFileSystemEntries settings.Directory + enumerateCacheFiles() |> Seq.iter(fun entry -> let entryName = Path.GetFileName entry @@ -83,30 +118,6 @@ type FileCache(logger: ILogger, ) } - let deleteFileSafe (fileInfo: FileInfo) = async { - if OperatingSystem.IsWindows() then - // NOTE: On older versions of Windows (known to reproduce on windows-2019 GitHub Actions image), the - // following scenario may be defunct: - // - open a file with FileShare.Delete (i.e. for download) - // - delete a file (i.e. during the cache cleanup) - // - try to create a file with the same name again - // - // According to this article - // (https://boostgsoc13.github.io/boost.afio/doc/html/afio/FAQ/deleting_open_files.html), it is impossible - // to do since file will occupy its disk name until the last handle is closed. - // - // In practice, this is allowed (checked at least on Windows 10 20H2 and windows-2022 GitHub Actions image), - // but is known to be broken on older versions of Windows (windows-2019). - // - // As a workaround, let's rename the file to a random name before deleting it. - // - // This workaround may be removed after these older versions of Windows goes out of support. - fileInfo.MoveTo(Path.Combine(fileInfo.DirectoryName, $"{Guid.NewGuid().ToString()}.deleted")) - fileInfo.Delete() - else - fileInfo.Delete() - } - let ensureFreeCache size = async { if size > settings.FileSizeLimitBytes || size > settings.TotalCacheSizeLimitBytes then return false @@ -114,9 +125,7 @@ type FileCache(logger: ILogger, do! assertCacheDirectoryExists() do! assertCacheValid() - let allEntries = - Directory.EnumerateFileSystemEntries settings.Directory - |> Seq.map FileInfo + let allEntries = enumerateCacheFiles() |> Seq.map FileInfo // Now, sort the entries from newest to oldest, and start deleting if required at a point when we understand // that there are too much files: diff --git a/Emulsion.Database/Migrations/20220828132228_UpdatedUniqueContentIndex.fs b/Emulsion.Database/Migrations/20220828132228_UpdatedUniqueContentIndex.fs new file mode 100644 index 00000000..4f79b532 --- /dev/null +++ b/Emulsion.Database/Migrations/20220828132228_UpdatedUniqueContentIndex.fs @@ -0,0 +1,63 @@ +// +namespace Emulsion.Database.Migrations + +open System +open Emulsion.Database +open Microsoft.EntityFrameworkCore +open Microsoft.EntityFrameworkCore.Infrastructure +open Microsoft.EntityFrameworkCore.Migrations + +[)>] +[] +type UpdatedUniqueContentIndex() = + inherit Migration() + + override this.Up(migrationBuilder:MigrationBuilder) = + migrationBuilder.Sql @" + drop index TelegramContents_Unique + + create unique index TelegramContents_Unique + on TelegramContents(ChatUserName, MessageId, FileId, FileName, MimeType) + " |> ignore + + override this.Down(migrationBuilder:MigrationBuilder) = + migrationBuilder.Sql @" + drop index TelegramContents_Unique + + create unique index TelegramContents_Unique + on TelegramContents(ChatUserName, MessageId, FileId) + " |> ignore + + override this.BuildTargetModel(modelBuilder: ModelBuilder) = + modelBuilder + .HasAnnotation("ProductVersion", "5.0.10") + |> ignore + + modelBuilder.Entity("Emulsion.Database.Entities.TelegramContent", (fun b -> + + b.Property("Id") + .IsRequired(true) + .ValueGeneratedOnAdd() + .HasColumnType("INTEGER") |> ignore + b.Property("ChatUserName") + .IsRequired(false) + .HasColumnType("TEXT") |> ignore + b.Property("FileId") + .IsRequired(false) + .HasColumnType("TEXT") |> ignore + b.Property("FileName") + .IsRequired(false) + .HasColumnType("TEXT") |> ignore + b.Property("MessageId") + .IsRequired(true) + .HasColumnType("INTEGER") |> ignore + b.Property("MimeType") + .IsRequired(false) + .HasColumnType("TEXT") |> ignore + + b.HasKey("Id") |> ignore + + b.ToTable("TelegramContents") |> ignore + + )) |> ignore + diff --git a/Emulsion.Tests/ContentProxy/FileCacheTests.fs b/Emulsion.Tests/ContentProxy/FileCacheTests.fs index 3fbb21f7..0548d17f 100644 --- a/Emulsion.Tests/ContentProxy/FileCacheTests.fs +++ b/Emulsion.Tests/ContentProxy/FileCacheTests.fs @@ -25,6 +25,10 @@ type FileCacheTests(outputHelper: ITestOutputHelper) = let assertCacheState(entries: (string * byte[]) seq) = let files = Directory.EnumerateFileSystemEntries(cacheDirectory.Value) + |> Seq.filter(fun f -> + if FileCache.IsMoveAndDeleteModeEnabled then not(f.EndsWith ".deleted") + else true + ) |> Seq.map(fun file -> let name = Path.GetFileName file let content = File.ReadAllBytes file