unkin/jellyfin-ha-src
4
0
Fork 0
Code Issues 10 Pull Requests 2 Packages Projects Releases Wiki Activity

Reduce comments

Browse Source
This commit is contained in:
Shadowghost
2026-09-03 11:51:02 +02:00
parent d73e3d964e
commit b724e57458
9 changed files with 28 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Emby.Server.Implementations/IO/LibraryMonitor.cs
+2 -4
View File
@@ -368,10 +368,8 @@ namespace Emby.Server.Implementations.IO
return;
}
// Something changed on disk that the server did not necessarily do itself, so whatever is
// cached about that path and its folder is now a guess. This sits above the checks below
// because a change we deliberately do not refresh for still has to be read correctly the
// next time somebody looks at that folder.
// Invalidate before the checks below: a change we deliberately do not refresh for still
// has to be read correctly the next time somebody looks at that folder.
_directoryService.Invalidate(path);
// Ignore certain files, If the parent of an ignored path has a change event, ignore that too
Emby.Server.Implementations/Library/LibraryManager.cs
+2 -2
View File
@@ -3723,8 +3723,8 @@ namespace Emby.Server.Implementations.Library
}
}
// The directory caches are shared, so the validation below would otherwise resolve
// the libraries root from a listing taken before this folder was created.
// The validation below would otherwise resolve the libraries root from a listing
// taken before this folder was created.
_directoryService.Invalidate(virtualFolderPath);
}
finally
Jellyfin.Api/Controllers/LibraryStructureController.cs
+2 -2
View File
@@ -189,8 +189,8 @@ public class LibraryStructureController : BaseJellyfinApiController
Directory.Move(currentPath, newPath);
// The directory caches are shared, so the validation below would otherwise resolve the
// libraries root from a listing taken before the folder was moved.
// The validation below would otherwise resolve the libraries root from a listing taken
// before the folder was moved.
_directoryService.Invalidate(currentPath);
_directoryService.Invalidate(newPath);
}
MediaBrowser.Controller/Providers/DirectoryService.cs
+4 -10
View File
@@ -91,8 +91,7 @@ namespace MediaBrowser.Controller.Providers
{
var file = _fileSystem.GetFileSystemInfo(path);
// Only a hit is remembered. A miss is the one answer that changes on its own, when
// the file the path names turns up.
// Only cache hits: a missing file can turn up later.
if (file?.Exists ?? false)
{
result = file;
@@ -110,8 +109,7 @@ namespace MediaBrowser.Controller.Providers
{
if (clearCache)
{
// Only what is remembered about this directory. Invalidate() also drops the parent,
// which a write needs but which is needless churn on a shared cache here.
// Not Invalidate(), which would also drop the parent listing for no reason here.
Forget(path);
}
@@ -133,9 +131,6 @@ namespace MediaBrowser.Controller.Providers
public void Invalidate(string path)
{
// Everything remembered about the path itself, and the listing of the directory holding
// it, since writing a file changes what its directory contains. The caches belong to the
// file system rather than to this instance, so this is felt by every reader of it.
Forget(path);
var parent = Path.GetDirectoryName(path);
@@ -162,9 +157,8 @@ namespace MediaBrowser.Controller.Providers
private const int DirectoryCacheSize = 2048;
private const int FileCacheSize = 8192;
// A DirectoryService no longer bounds how long its answers are trusted by dying, so a
// lifetime does. This is a staleness bound, not a snapshot: a long refresh can outlive it
// and re-read a directory partway through.
// The cache outlives the DirectoryService instances reading it, so entries need their
// own staleness bound. A long refresh can outlive it and re-read a directory partway.
private static readonly TimeSpan _entryLifetime = TimeSpan.FromMinutes(1);
public ConcurrentTLru<string, FileSystemMetadata[]> Entries { get; }
MediaBrowser.Providers/Lyric/LyricManager.cs
+2 -5
View File
@@ -255,8 +255,7 @@ public class LyricManager : ILyricManager
_libraryMonitor.ReportFileSystemChangeComplete(path, false);
}
// The refresh below reads the containing folder to find external lyrics, and would find
// the deleted one again in a cached listing.
// The refresh below would otherwise find the deleted file in a cached listing.
_directoryService.Invalidate(path);
}
@@ -454,9 +453,7 @@ public class LyricManager : ILyricManager
await stream.CopyToAsync(fs).ConfigureAwait(false);
}
// The directory caches are shared and outlive this call, so the refresh that follows
// would otherwise resolve external lyrics from a listing taken before this file
// landed.
// The refresh that follows would otherwise not see the new file.
_directoryService.Invalidate(savePath);
return;
MediaBrowser.Providers/Manager/ProviderManager.cs
+7 -10
View File
@@ -1143,8 +1143,8 @@ namespace MediaBrowser.Providers.Manager
return;
}
// PriorityQueue is not thread safe, and this runs on whichever thread queued the refresh
// while the processor dequeues on its own, so every touch of the queue takes the lock.
// PriorityQueue is not thread safe and the processor dequeues concurrently, so every
// touch of the queue takes the lock.
lock (_refreshQueueLock)
{
_refreshQueue.Enqueue((itemId, options), priority);
@@ -1182,10 +1182,8 @@ namespace MediaBrowser.Providers.Manager
{
(Guid ItemId, MetadataRefreshOptions RefreshOptions) refreshItem;
// Standing down and taking the next entry happen under one lock, and this is the
// only place the flag is handed back. Releasing it anywhere else would leave a gap
// in which a refresh queued just after the queue ran dry sees a processor that has
// already stopped, and waits forever.
// Dequeueing and standing down happen under one lock, otherwise a refresh queued
// just after the queue ran dry would see a processor that has already stopped.
lock (_refreshQueueLock)
{
if (_disposed
@@ -1213,14 +1211,13 @@ namespace MediaBrowser.Providers.Manager
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
// Shutting down. Whatever is still queued keeps its place; the next pass round
// stands the processor down, so the next refresh queued starts one of its own.
// Shutting down: the next pass sees the token and stands the processor down.
continue;
}
catch (Exception ex)
{
// A provider that cancelled for its own reasons lands here too, an HTTP timeout
// above all. One unreachable metadata server must not stop the queue draining.
// Includes a provider that cancelled for its own reasons, such as an HTTP
// timeout, which must not stop the queue draining.
_logger.LogError(ex, "Error refreshing item");
}
}
MediaBrowser.Providers/Subtitles/SubtitleManager.cs
+2 -5
View File
@@ -284,9 +284,7 @@ namespace MediaBrowser.Providers.Subtitles
await stream.CopyToAsync(fs).ConfigureAwait(false);
}
// The directory caches are shared and outlive this call, so the refresh that
// follows would otherwise resolve external subtitles from a listing taken
// before this file landed.
// The refresh that follows would otherwise not see the new file.
_directoryService.Invalidate(path);
return;
@@ -403,8 +401,7 @@ namespace MediaBrowser.Providers.Subtitles
_monitor.ReportFileSystemChangeComplete(path, false);
}
// The refresh below reads the containing folder to find external subtitles, and would
// find the deleted one again in a cached listing.
// The refresh below would otherwise find the deleted file in a cached listing.
_directoryService.Invalidate(path);
return item.RefreshMetadata(CancellationToken.None);
tests/Jellyfin.Controller.Tests/DirectoryServiceTests.cs
+1 -13
View File
@@ -268,9 +268,7 @@ namespace Jellyfin.Controller.Tests
[Fact]
public void GetFileSystemEntries_FarMorePathsThanTheCacheHolds_EvictsInsteadOfGrowing()
{
// The cache outlives every DirectoryService that reads it, so it has to give entries back
// rather than hold every path the server ever saw. Asking for more paths than it can hold
// must push the first one out, which shows up as the file system being read for it twice.
// Eviction of the first path shows up as the file system being read for it twice.
const int PathCount = 8192;
var fileSystemMock = new Mock<IFileSystem>();
@@ -295,8 +293,6 @@ namespace Jellyfin.Controller.Tests
[Fact]
public void GetFileSystemEntries_SecondServiceOverSameFileSystem_ReusesTheFirstAnswer()
{
// The library code news up a DirectoryService per item, so what one of them learned about
// a directory has to be worth something to the next one reading the same file system.
var fileSystemMock = new Mock<IFileSystem>();
fileSystemMock.Setup(f => f.GetFileSystemEntries(LowerCasePath))
.Returns(_lowerCaseFileSystemMetadata);
@@ -328,8 +324,6 @@ namespace Jellyfin.Controller.Tests
[Fact]
public void Invalidate_GivenADirectory_DropsBothTheListingAndTheFilePaths()
{
// Clearing only one of the two views of a directory leaves the other one answering from
// before whatever was just written there.
var fileSystemMock = new Mock<IFileSystem>();
fileSystemMock.SetupSequence(f => f.GetFileSystemEntries(LowerCasePath))
.Returns(_lowerCaseFileSystemMetadata)
@@ -351,7 +345,6 @@ namespace Jellyfin.Controller.Tests
[Fact]
public void Invalidate_GivenAFile_DropsTheListingOfTheDirectoryHoldingIt()
{
// Downloading a subtitle changes what its folder contains, not just the one path.
const string NewFile = LowerCasePath + "/Song 2.srt";
var fileSystemMock = new Mock<IFileSystem>();
@@ -370,8 +363,6 @@ namespace Jellyfin.Controller.Tests
[Fact]
public void Invalidate_OnOneService_IsSeenByAnotherOverTheSameFileSystem()
{
// Whoever writes the file and whoever refreshes the item hold different services, so
// invalidating has to reach the cache both of them read.
var fileSystemMock = new Mock<IFileSystem>();
fileSystemMock.SetupSequence(f => f.GetFileSystemEntries(LowerCasePath))
.Returns(_lowerCaseFileSystemMetadata)
@@ -388,8 +379,6 @@ namespace Jellyfin.Controller.Tests
[Fact]
public void GetFilePaths_ClearingTheCache_KeepsTheParentDirectory()
{
// Re-reading one directory is not a reason to make the server list the library folder
// holding it again, which the shared cache would otherwise have to do.
const string ParentPath = "/music";
var fileSystemMock = new Mock<IFileSystem>();
@@ -421,7 +410,6 @@ namespace Jellyfin.Controller.Tests
Assert.Null(directoryService.GetFileSystemEntry(MissingPath));
// The one answer that changes on its own: the file turning up has to be visible.
Assert.NotNull(directoryService.GetFileSystemEntry(MissingPath));
}
}
tests/Jellyfin.Providers.Tests/Manager/ProviderManagerTests.cs
+6 -12
View File
@@ -381,10 +381,6 @@ namespace Jellyfin.Providers.Tests.Manager
[Fact]
public async Task QueueRefresh_ManyItemsQueuedFromManyThreads_ProcessesEveryOne()
{
// The queue is filled from whichever thread wants a refresh and drained by a processor of
// its own, so an unsynchronised PriorityQueue can lose entries outright, and a processor
// that stands down before releasing its flag leaves whatever was queued in that gap with
// nobody to drain it. Either way an item silently never gets refreshed.
const int ItemCount = 2000;
var queued = Enumerable.Range(0, ItemCount).Select(_ => Guid.NewGuid()).ToArray();
@@ -395,7 +391,7 @@ namespace Jellyfin.Providers.Tests.Manager
libraryManager.Setup(i => i.GetItemById(It.IsAny<Guid>()))
.Returns((Guid id) =>
{
// Returning null drains the entry without needing the whole refresh machinery.
// Returning null drains the entry without the whole refresh machinery.
processed.Add(id);
if (processed.Count == ItemCount)
{
@@ -425,7 +421,7 @@ namespace Jellyfin.Providers.Tests.Manager
}
catch (OperationCanceledException)
{
// Fall through, so the assertions below name what was lost rather than the wait.
// Fall through so the assertions report what was lost.
}
Assert.Empty(providerManager.GetRefreshQueue());
@@ -435,9 +431,8 @@ namespace Jellyfin.Providers.Tests.Manager
[Fact]
public async Task QueueRefresh_RefreshCancelsForItsOwnReasons_KeepsDrainingTheQueue()
{
// MetadataService rethrows OperationCanceledException out of a provider, so an HTTP
// timeout against an unreachable metadata server arrives here looking exactly like a
// shutdown. Treating it as one stops the processor and strands the rest of the queue.
// A provider timeout arrives as an OperationCanceledException, indistinguishable from
// a shutdown; treating it as one would strand the rest of the queue.
const int ItemCount = 200;
var queued = Enumerable.Range(0, ItemCount).Select(_ => Guid.NewGuid()).ToArray();
@@ -454,8 +449,7 @@ namespace Jellyfin.Providers.Tests.Manager
{
cancelledOnce = true;
// Hold the first entry until the whole batch is queued, so everything that
// follows it is already waiting when the cancellation lands.
// Hold the first entry until the whole batch is queued.
allQueued.Wait(TimeSpan.FromSeconds(30));
throw new OperationCanceledException("provider timed out");
}
@@ -487,7 +481,7 @@ namespace Jellyfin.Providers.Tests.Manager
}
catch (OperationCanceledException)
{
// Fall through, so the assertions below name what was left stranded.
// Fall through so the assertions report what was stranded.
}
Assert.Empty(providerManager.GetRefreshQueue());