Merge pull request #16941 from Shadowghost/fix-external-data-pruning

Fix external data pruning on item deletion

6 files changed, 236 insertions, 19 deletions

diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs
index c81829688f..7cbff0c67e 100644
--- a/Emby.Server.Implementations/ApplicationHost.cs
+++ b/Emby.Server.Implementations/ApplicationHost.cs
@@ -539,6 +539,7 @@ namespace Emby.Server.Implementations
             serviceCollection.AddTransient(provider => new Lazy<ILibraryMonitor>(provider.GetRequiredService<ILibraryMonitor>));
             serviceCollection.AddTransient(provider => new Lazy<IProviderManager>(provider.GetRequiredService<IProviderManager>));
             serviceCollection.AddTransient(provider => new Lazy<IUserViewManager>(provider.GetRequiredService<IUserViewManager>));
+            serviceCollection.AddTransient(provider => new Lazy<IExternalDataManager>(provider.GetRequiredService<IExternalDataManager>));
             serviceCollection.AddSingleton<ILibraryManager, LibraryManager>();
             serviceCollection.AddSingleton<NamingOptions>();
             serviceCollection.AddSingleton<VideoListResolver>();
diff --git a/Emby.Server.Implementations/Library/ExternalDataManager.cs b/Emby.Server.Implementations/Library/ExternalDataManager.cs
index 4ad0f999bf..2c18e56df7 100644
--- a/Emby.Server.Implementations/Library/ExternalDataManager.cs
+++ b/Emby.Server.Implementations/Library/ExternalDataManager.cs
@@ -1,6 +1,5 @@
 using System;
 using System.IO;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using MediaBrowser.Controller.Chapters;
@@ -52,26 +51,33 @@ public class ExternalDataManager : IExternalDataManager
     /// <inheritdoc/>
     public async Task DeleteExternalItemDataAsync(BaseItem item, CancellationToken cancellationToken)
     {
-        var validPaths = _pathManager.GetExtractedDataPaths(item).Where(Directory.Exists).ToList();
-        var itemId = item.Id;
-        if (validPaths.Count > 0)
-        {
-            foreach (var path in validPaths)
-            {
-                try
-                {
-                    Directory.Delete(path, true);
-                }
-                catch (Exception ex)
-                {
-                    _logger.LogWarning("Unable to prune external item data at {Path}: {Exception}", path, ex);
-                }
-            }
-        }
+        DeleteExternalItemFiles(item);
 
+        var itemId = item.Id;
         await _keyframeManager.DeleteKeyframeDataAsync(itemId, cancellationToken).ConfigureAwait(false);
         await _mediaSegmentManager.DeleteSegmentsAsync(itemId, cancellationToken).ConfigureAwait(false);
         await _trickplayManager.DeleteTrickplayDataAsync(itemId, cancellationToken).ConfigureAwait(false);
         await _chapterManager.DeleteChapterDataAsync(itemId, cancellationToken).ConfigureAwait(false);
     }
+
+    /// <inheritdoc/>
+    public void DeleteExternalItemFiles(BaseItem item)
+    {
+        foreach (var path in _pathManager.GetExtractedDataPaths(item))
+        {
+            if (!Directory.Exists(path))
+            {
+                continue;
+            }
+
+            try
+            {
+                Directory.Delete(path, true);
+            }
+            catch (Exception ex)
+            {
+                _logger.LogWarning("Unable to prune external item data at {Path}: {Exception}", path, ex);
+            }
+        }
+    }
 }
diff --git a/Emby.Server.Implementations/Library/LibraryManager.cs b/Emby.Server.Implementations/Library/LibraryManager.cs
index ffc449d974..6ed417c395 100644
--- a/Emby.Server.Implementations/Library/LibraryManager.cs
+++ b/Emby.Server.Implementations/Library/LibraryManager.cs
@@ -89,6 +89,7 @@ namespace Emby.Server.Implementations.Library
         private readonly FastConcurrentLru<Guid, BaseItem> _cache;
         private readonly DotIgnoreIgnoreRule _dotIgnoreIgnoreRule;
         private readonly IMediaStreamRepository _mediaStreamRepository;
+        private readonly Lazy<IExternalDataManager> _externalDataManagerFactory;
 
         /// <summary>
         /// The _root folder sync lock.
@@ -132,6 +133,7 @@ namespace Emby.Server.Implementations.Library
         /// <param name="pathManager">The path manager.</param>
         /// <param name="dotIgnoreIgnoreRule">The .ignore rule handler.</param>
         /// <param name="mediaStreamRepository">The media stream repository.</param>
+        /// <param name="externalDataManagerFactory">The external data manager (lazy, to break the DI cycle through ChapterManager).</param>
         public LibraryManager(
             IServerApplicationHost appHost,
             ILoggerFactory loggerFactory,
@@ -155,7 +157,8 @@ namespace Emby.Server.Implementations.Library
             IPeopleRepository peopleRepository,
             IPathManager pathManager,
             DotIgnoreIgnoreRule dotIgnoreIgnoreRule,
-            IMediaStreamRepository mediaStreamRepository)
+            IMediaStreamRepository mediaStreamRepository,
+            Lazy<IExternalDataManager> externalDataManagerFactory)
         {
             _appHost = appHost;
             _logger = loggerFactory.CreateLogger<LibraryManager>();
@@ -186,6 +189,7 @@ namespace Emby.Server.Implementations.Library
             _configurationManager.ConfigurationUpdated += ConfigurationUpdated;
 
             _mediaStreamRepository = mediaStreamRepository;
+            _externalDataManagerFactory = externalDataManagerFactory;
 
             RecordConfigurationValues(_configurationManager.Configuration);
         }
@@ -396,6 +400,12 @@ namespace Emby.Server.Implementations.Library
                 }
             }
 
+            var externalDataManager = _externalDataManagerFactory.Value;
+            foreach (var (item, _, _) in pathMaps)
+            {
+                externalDataManager.DeleteExternalItemFiles(item);
+            }
+
             _persistenceService.DeleteItem([.. pathMaps.Select(f => f.Item.Id)]);
         }
 
@@ -576,6 +586,13 @@ namespace Emby.Server.Implementations.Library
 
             item.SetParent(null);
 
+            var externalDataManager = _externalDataManagerFactory.Value;
+            externalDataManager.DeleteExternalItemFiles(item);
+            foreach (var child in children)
+            {
+                externalDataManager.DeleteExternalItemFiles(child);
+            }
+
             _persistenceService.DeleteItem([item.Id, .. children.Select(f => f.Id)]);
             _cache.TryRemove(item.Id, out _);
             foreach (var child in children)
diff --git a/Emby.Server.Implementations/Library/PathManager.cs b/Emby.Server.Implementations/Library/PathManager.cs
index ef5edb9afa..fad948ad97 100644
--- a/Emby.Server.Implementations/Library/PathManager.cs
+++ b/Emby.Server.Implementations/Library/PathManager.cs
@@ -121,7 +121,11 @@ public class PathManager : IPathManager
         }
 
         paths.Add(GetTrickplayDirectory(item, false));
-        paths.Add(GetTrickplayDirectory(item, true));
+        if (!string.IsNullOrEmpty(item.Path))
+        {
+            paths.Add(GetTrickplayDirectory(item, true));
+        }
+
         paths.Add(GetChapterImageFolderPath(item));
 
         return paths;
diff --git a/Jellyfin.Server/Migrations/Routines/20260525010000_CleanupOrphanedExternalData.cs b/Jellyfin.Server/Migrations/Routines/20260525010000_CleanupOrphanedExternalData.cs
new file mode 100644
index 0000000000..d8dfe181ca
--- /dev/null
+++ b/Jellyfin.Server/Migrations/Routines/20260525010000_CleanupOrphanedExternalData.cs
@@ -0,0 +1,182 @@
+using System;
+using System.Collections.Generic;
+using System.Globalization;
+using System.IO;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using Jellyfin.Database.Implementations;
+using Jellyfin.Server.ServerSetupApp;
+using MediaBrowser.Common.Configuration;
+using MediaBrowser.Controller;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.Logging;
+
+namespace Jellyfin.Server.Migrations.Routines;
+
+/// <summary>
+/// Removes on-disk external item data (attachments, subtitles, trickplay tiles, chapter images) for items that
+/// no longer exist in the <c>BaseItems</c> table. The database side is cleaned up synchronously by
+/// <c>IItemPersistenceService.DeleteItem</c>, so the leftover orphans live on the filesystem.
+/// </summary>
+[JellyfinMigration("2026-05-25T01:00:00", nameof(CleanupOrphanedExternalData))]
+[JellyfinMigrationBackup(JellyfinDb = true)]
+public class CleanupOrphanedExternalData : IAsyncMigrationRoutine
+{
+    private const int ProgressLogStep = 500;
+
+    private readonly IStartupLogger<CleanupOrphanedExternalData> _logger;
+    private readonly IDbContextFactory<JellyfinDbContext> _dbContextFactory;
+    private readonly IApplicationPaths _appPaths;
+    private readonly IServerApplicationPaths _serverPaths;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="CleanupOrphanedExternalData"/> class.
+    /// </summary>
+    /// <param name="logger">The startup logger.</param>
+    /// <param name="dbContextFactory">The database context factory.</param>
+    /// <param name="appPaths">The application paths.</param>
+    /// <param name="serverPaths">The server application paths.</param>
+    public CleanupOrphanedExternalData(
+        IStartupLogger<CleanupOrphanedExternalData> logger,
+        IDbContextFactory<JellyfinDbContext> dbContextFactory,
+        IApplicationPaths appPaths,
+        IServerApplicationPaths serverPaths)
+    {
+        _logger = logger;
+        _dbContextFactory = dbContextFactory;
+        _appPaths = appPaths;
+        _serverPaths = serverPaths;
+    }
+
+    /// <inheritdoc/>
+    public async Task PerformAsync(CancellationToken cancellationToken)
+    {
+        var knownIds = await LoadKnownItemIdsAsync(cancellationToken).ConfigureAwait(false);
+
+        CleanupGuidIndexedRoot(
+            "attachment",
+            Path.Combine(_appPaths.DataPath, "attachments"),
+            knownIds,
+            deleteSubPath: null,
+            cancellationToken);
+
+        CleanupGuidIndexedRoot(
+            "subtitle",
+            Path.Combine(_appPaths.DataPath, "subtitles"),
+            knownIds,
+            deleteSubPath: null,
+            cancellationToken);
+
+        CleanupGuidIndexedRoot(
+            "trickplay",
+            _appPaths.TrickplayPath,
+            knownIds,
+            deleteSubPath: null,
+            cancellationToken);
+
+        CleanupGuidIndexedRoot(
+            "chapter image",
+            Path.Combine(_serverPaths.InternalMetadataPath, "library"),
+            knownIds,
+            deleteSubPath: "chapters",
+            cancellationToken);
+    }
+
+    private async Task<HashSet<Guid>> LoadKnownItemIdsAsync(CancellationToken cancellationToken)
+    {
+        var context = await _dbContextFactory.CreateDbContextAsync(cancellationToken).ConfigureAwait(false);
+        await using (context.ConfigureAwait(false))
+        {
+            var ids = await context.BaseItems
+                .AsNoTracking()
+                .Select(b => b.Id)
+                .ToListAsync(cancellationToken)
+                .ConfigureAwait(false);
+            return [.. ids];
+        }
+    }
+
+    private void CleanupGuidIndexedRoot(
+        string label,
+        string root,
+        HashSet<Guid> knownIds,
+        string? deleteSubPath,
+        CancellationToken cancellationToken)
+    {
+        if (string.IsNullOrEmpty(root) || !Directory.Exists(root))
+        {
+            _logger.LogInformation("Skipping {Label} cleanup; root {Root} does not exist", label, root);
+            return;
+        }
+
+        _logger.LogInformation("Scanning for orphaned {Label} data under {Root}", label, root);
+
+        var scanned = 0;
+        var removed = 0;
+        foreach (var prefixDir in Directory.EnumerateDirectories(root))
+        {
+            cancellationToken.ThrowIfCancellationRequested();
+
+            var prefixName = Path.GetFileName(prefixDir);
+            if (prefixName.Length != 2)
+            {
+                continue;
+            }
+
+            foreach (var guidDir in Directory.EnumerateDirectories(prefixDir))
+            {
+                cancellationToken.ThrowIfCancellationRequested();
+
+                scanned++;
+                if (scanned % ProgressLogStep == 0)
+                {
+                    _logger.LogInformation("Scanning {Label}: {Scanned} directories examined, {Removed} orphans removed so far", label, scanned, removed);
+                }
+
+                var leafName = Path.GetFileName(guidDir);
+                if (!Guid.TryParse(leafName, CultureInfo.InvariantCulture, out var id))
+                {
+                    continue;
+                }
+
+                if (knownIds.Contains(id))
+                {
+                    continue;
+                }
+
+                var target = deleteSubPath is null ? guidDir : Path.Combine(guidDir, deleteSubPath);
+                if (deleteSubPath is not null && !Directory.Exists(target))
+                {
+                    continue;
+                }
+
+                if (TryDelete(target))
+                {
+                    removed++;
+                }
+            }
+        }
+
+        _logger.LogInformation("Finished {Label} cleanup: scanned {Scanned} directories, removed {Removed} orphans", label, scanned, removed);
+    }
+
+    private bool TryDelete(string dir)
+    {
+        try
+        {
+            Directory.Delete(dir, recursive: true);
+            return true;
+        }
+        catch (IOException ex)
+        {
+            _logger.LogWarning(ex, "Failed to delete orphaned directory {Dir}", dir);
+        }
+        catch (UnauthorizedAccessException ex)
+        {
+            _logger.LogWarning(ex, "Permission denied deleting orphaned directory {Dir}", dir);
+        }
+
+        return false;
+    }
+}
diff --git a/MediaBrowser.Controller/IO/IExternalDataManager.cs b/MediaBrowser.Controller/IO/IExternalDataManager.cs
index f69f4586c6..b2eb8fc3f1 100644
--- a/MediaBrowser.Controller/IO/IExternalDataManager.cs
+++ b/MediaBrowser.Controller/IO/IExternalDataManager.cs
@@ -16,4 +16,11 @@ public interface IExternalDataManager
     /// <param name="cancellationToken">The cancellation token.</param>
     /// <returns>Task.</returns>
     Task DeleteExternalItemDataAsync(BaseItem item, CancellationToken cancellationToken);
+
+    /// <summary>
+    /// Deletes only the filesystem-side external item data (attachments, subtitles, trickplay, chapter images).
+    /// Use this when DB-side cleanup is already handled by another code path (e.g. <c>IItemPersistenceService.DeleteItem</c>).
+    /// </summary>
+    /// <param name="item">The item.</param>
+    void DeleteExternalItemFiles(BaseItem item);
 }