Add lease-aware cleanup to DeleteTranscodeFileTask (#25)

* Initial plan * Add GetActiveSessionsAsync to ITranscodeSessionStore and update DeleteTranscodeFileTask for lease-aware cleanup Co-authored-by: ZoltyMat <177592743+ZoltyMat@users.noreply.github.com> * Fix Redis exception propagation in GetActiveSessionsAsync for safe abort behavior Co-authored-by: ZoltyMat <177592743+ZoltyMat@users.noreply.github.com> * fix: use KeysAsync to resolve CA1849 analyzer violation Replace synchronous IServer.Keys() with async IServer.KeysAsync() using await foreach to satisfy CA1849 (TreatWarningsAsErrors). CA1849: 'IServer.Keys()' synchronously blocks. Await 'IServer.KeysAsync()' instead. Line 161 in RedisTranscodeSessionStore.GetActiveSessionsAsync. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ZoltyMat <177592743+ZoltyMat@users.noreply.github.com> Co-authored-by: mat <mstrommen@gmail.com>
2026-03-10 01:11:32 -04:00
parent c2a11f3e68
commit 9817185fa3
9 changed files with 412 additions and 6 deletions
@@ -1,8 +1,13 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using MediaBrowser.Common.Configuration;
 using MediaBrowser.Controller.MediaEncoding;
+using MediaBrowser.Model.Configuration;
+using MediaBrowser.Model.IO;
+using Moq;
 using Xunit;

 namespace Jellyfin.Server.Implementations.Tests.ScheduledTasks;
@@ -31,6 +36,27 @@ public class DeleteTranscodeFileTaskTests
            LastDurablePlaybackOffset = 12_000_000L,
        };

+    /// <summary>
+    /// Creates a mock <see cref="IConfigurationManager"/> that returns <paramref name="transcodePath"/>
+    /// as the configured transcode path, used by the <c>GetTranscodePath</c> extension method.
+    /// </summary>
+    private static Mock<IConfigurationManager> CreateConfigMock(string transcodePath)
+    {
+        var appPathsMock = new Mock<IApplicationPaths>();
+        appPathsMock
+            .Setup(p => p.CreateAndCheckMarker(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<bool>()));
+
+        var configMock = new Mock<IConfigurationManager>();
+        configMock
+            .Setup(c => c.GetConfiguration("encoding"))
+            .Returns(new EncodingOptions { TranscodingTempPath = transcodePath });
+        configMock
+            .Setup(c => c.CommonApplicationPaths)
+            .Returns(appPathsMock.Object);
+
+        return configMock;
+    }
+
    /// <summary>
    /// A directory that belongs to a session with a live lease must NOT be deleted.
    /// The store returns non-null, signalling to the cleanup task that the session is active.
@@ -87,6 +113,206 @@ public class DeleteTranscodeFileTaskTests
        Assert.Null(liveSession);
    }

+    /// <summary>
+    /// Files that belong to an active session (manifest or segments) must NOT be deleted
+    /// even when their modification time is older than <c>minDateModified</c>.
+    /// </summary>
+    [Fact]
+    [Trait("Category", "UnitTest")]
+    public async Task ExecuteAsync_WithActiveSession_DoesNotDeleteActiveFiles()
+    {
+        // Arrange
+        const string TranscodePath = "/transcode";
+        const string SessionId = "active-session-1";
+        const string ManifestPath = "/transcode/active-session-1/manifest.m3u8";
+        const string SegmentPath = "/transcode/active-session-1/segment0.ts";
+
+        var store = new CleanupTestSessionStore();
+        var session = new TranscodeSession
+        {
+            PlaySessionId = SessionId,
+            OwnerPod = "pod-a",
+            LeaseExpiresUtc = DateTime.UtcNow.AddMinutes(5),
+            ManifestPath = ManifestPath,
+            SegmentPathPrefix = "/transcode/active-session-1/segment",
+            MediaSourceId = "media-source-1",
+        };
+        await store.SetAsync(session);
+
+        var deletedFiles = new List<string>();
+        var oldModifyTime = DateTime.UtcNow.AddDays(-2);
+
+        var fileSystemMock = new Mock<IFileSystem>();
+        fileSystemMock
+            .Setup(fs => fs.GetFiles(TranscodePath, true))
+            .Returns(new[]
+            {
+                new FileSystemMetadata { FullName = ManifestPath, IsDirectory = false },
+                new FileSystemMetadata { FullName = SegmentPath, IsDirectory = false },
+            });
+        fileSystemMock
+            .Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
+            .Returns(oldModifyTime);
+        fileSystemMock
+            .Setup(fs => fs.DeleteFile(It.IsAny<string>()))
+            .Callback<string>(path => deletedFiles.Add(path));
+        fileSystemMock
+            .Setup(fs => fs.GetFiles(TranscodePath, false))
+            .Returns(Enumerable.Empty<FileSystemMetadata>());
+        fileSystemMock
+            .Setup(fs => fs.GetDirectories(It.IsAny<string>(), It.IsAny<bool>()))
+            .Returns(Enumerable.Empty<FileSystemMetadata>());
+
+        var configMock = CreateConfigMock(TranscodePath);
+
+        var localizationMock = new Mock<MediaBrowser.Model.Globalization.ILocalizationManager>();
+        localizationMock
+            .Setup(l => l.GetLocalizedString(It.IsAny<string>()))
+            .Returns<string>(s => s);
+
+        var loggerMock = new Mock<Microsoft.Extensions.Logging.ILogger<Emby.Server.Implementations.ScheduledTasks.Tasks.DeleteTranscodeFileTask>>();
+
+        var task = new Emby.Server.Implementations.ScheduledTasks.Tasks.DeleteTranscodeFileTask(
+            loggerMock.Object,
+            fileSystemMock.Object,
+            configMock.Object,
+            localizationMock.Object,
+            store);
+
+        // Act
+        await task.ExecuteAsync(new Progress<double>(), CancellationToken.None);
+
+        // Assert – neither the manifest nor the segment should have been deleted
+        Assert.DoesNotContain(ManifestPath, deletedFiles);
+        Assert.DoesNotContain(SegmentPath, deletedFiles);
+    }
+
+    /// <summary>
+    /// Files whose session lease has expired are NOT returned by <see cref="ITranscodeSessionStore.GetActiveSessionsAsync"/>
+    /// and therefore should be eligible for time-based deletion.
+    /// </summary>
+    [Fact]
+    [Trait("Category", "UnitTest")]
+    public async Task ExecuteAsync_WithExpiredSession_DeletesFiles()
+    {
+        // Arrange
+        const string TranscodePath = "/transcode";
+        const string SessionId = "expired-session-1";
+        const string ManifestPath = "/transcode/expired-session-1/manifest.m3u8";
+
+        var store = new CleanupTestSessionStore();
+        // Lease expired two hours ago
+        var session = new TranscodeSession
+        {
+            PlaySessionId = SessionId,
+            OwnerPod = "pod-a",
+            LeaseExpiresUtc = DateTime.UtcNow.AddHours(-2),
+            ManifestPath = ManifestPath,
+            SegmentPathPrefix = "/transcode/expired-session-1/segment",
+            MediaSourceId = "media-source-1",
+        };
+        await store.SetAsync(session);
+
+        var deletedFiles = new List<string>();
+        var oldModifyTime = DateTime.UtcNow.AddDays(-2);
+
+        var fileSystemMock = new Mock<IFileSystem>();
+        fileSystemMock
+            .Setup(fs => fs.GetFiles(TranscodePath, true))
+            .Returns(new[]
+            {
+                new FileSystemMetadata { FullName = ManifestPath, IsDirectory = false },
+            });
+        fileSystemMock
+            .Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
+            .Returns(oldModifyTime);
+        fileSystemMock
+            .Setup(fs => fs.DeleteFile(It.IsAny<string>()))
+            .Callback<string>(path => deletedFiles.Add(path));
+        fileSystemMock
+            .Setup(fs => fs.GetDirectories(It.IsAny<string>(), It.IsAny<bool>()))
+            .Returns(Enumerable.Empty<FileSystemMetadata>());
+
+        var configMock = CreateConfigMock(TranscodePath);
+
+        var localizationMock = new Mock<MediaBrowser.Model.Globalization.ILocalizationManager>();
+        localizationMock
+            .Setup(l => l.GetLocalizedString(It.IsAny<string>()))
+            .Returns<string>(s => s);
+
+        var loggerMock = new Mock<Microsoft.Extensions.Logging.ILogger<Emby.Server.Implementations.ScheduledTasks.Tasks.DeleteTranscodeFileTask>>();
+
+        var task = new Emby.Server.Implementations.ScheduledTasks.Tasks.DeleteTranscodeFileTask(
+            loggerMock.Object,
+            fileSystemMock.Object,
+            configMock.Object,
+            localizationMock.Object,
+            store);
+
+        // Act
+        await task.ExecuteAsync(new Progress<double>(), CancellationToken.None);
+
+        // Assert – expired session files are eligible for time-based deletion
+        Assert.Contains(ManifestPath, deletedFiles);
+    }
+
+    /// <summary>
+    /// When <see cref="ITranscodeSessionStore.GetActiveSessionsAsync"/> throws an exception,
+    /// the task should abort deletion safely rather than risk removing files in use.
+    /// </summary>
+    [Fact]
+    [Trait("Category", "UnitTest")]
+    public async Task ExecuteAsync_WhenStoreFails_AbortsDeletion()
+    {
+        // Arrange
+        const string TranscodePath = "/transcode";
+        const string ManifestPath = "/transcode/session-1/manifest.m3u8";
+
+        var deletedFiles = new List<string>();
+        var oldModifyTime = DateTime.UtcNow.AddDays(-2);
+
+        var fileSystemMock = new Mock<IFileSystem>();
+        fileSystemMock
+            .Setup(fs => fs.GetFiles(TranscodePath, true))
+            .Returns(new[]
+            {
+                new FileSystemMetadata { FullName = ManifestPath, IsDirectory = false },
+            });
+        fileSystemMock
+            .Setup(fs => fs.GetLastWriteTimeUtc(It.IsAny<FileSystemMetadata>()))
+            .Returns(oldModifyTime);
+        fileSystemMock
+            .Setup(fs => fs.DeleteFile(It.IsAny<string>()))
+            .Callback<string>(path => deletedFiles.Add(path));
+
+        var configMock = CreateConfigMock(TranscodePath);
+
+        var localizationMock = new Mock<MediaBrowser.Model.Globalization.ILocalizationManager>();
+        localizationMock
+            .Setup(l => l.GetLocalizedString(It.IsAny<string>()))
+            .Returns<string>(s => s);
+
+        var loggerMock = new Mock<Microsoft.Extensions.Logging.ILogger<Emby.Server.Implementations.ScheduledTasks.Tasks.DeleteTranscodeFileTask>>();
+
+        var failingStoreMock = new Mock<ITranscodeSessionStore>();
+        failingStoreMock
+            .Setup(s => s.GetActiveSessionsAsync(It.IsAny<CancellationToken>()))
+            .ThrowsAsync(new InvalidOperationException("Redis unavailable"));
+
+        var task = new Emby.Server.Implementations.ScheduledTasks.Tasks.DeleteTranscodeFileTask(
+            loggerMock.Object,
+            fileSystemMock.Object,
+            configMock.Object,
+            localizationMock.Object,
+            failingStoreMock.Object);
+
+        // Act
+        await task.ExecuteAsync(new Progress<double>(), CancellationToken.None);
+
+        // Assert – when the store fails, no files should be deleted (safe abort)
+        Assert.Empty(deletedFiles);
+    }
+
    /// <summary>
    /// Minimal in-memory <see cref="ITranscodeSessionStore"/> used within this test class
    /// to avoid a cross-project reference to Jellyfin.MediaEncoding.Tests.
@@ -166,6 +392,18 @@ public class DeleteTranscodeFileTaskTests
            return Task.CompletedTask;
        }

+        public Task<IEnumerable<TranscodeSession>> GetActiveSessionsAsync(CancellationToken cancellationToken = default)
+        {
+            lock (_lock)
+            {
+                var sessions = _sessions.Values
+                    .Where(s => s.LeaseExpiresUtc > DateTime.UtcNow)
+                    .Select(Clone)
+                    .ToList();
+                return Task.FromResult<IEnumerable<TranscodeSession>>(sessions);
+            }
+        }
+
        private static TranscodeSession Clone(TranscodeSession source)
            => new TranscodeSession
            {