Final low-severity items from the re-review of ee10f932. L2: DbUpdateConcurrencyException derives from DbUpdateException but carries no provider exception, so IsUniqueConstraintViolation does not classify it. A row deleted by a concurrent library edit between the heal's read and its save would propagate and fail the scan, contradicting the invariant stated directly above it. Admit it in the filter. L1: lift the in-memory ordinal settle into LibraryRepository.ResolveExact and unit-test it with both spellings in the candidate list. No SQLite-backed test can exercise it (SQLite's = on TEXT is already binary), so this converts the half that rested on hand-run MySQL evidence into automated coverage. The end-to-end companion test's comment no longer claims to be provider-independent. L4: assert PRAGMA foreign_keys is 1 before migrating, so the enforcement guard cannot silently degrade into the weak pre-fix form it was added to replace. L3: detach the failed heal, matching the insert path. N3: the heal's inner predicate now matches its IsNullOrEmpty outer guard, so a PathHash = '' row cannot enter the branch and silently never heal. N4: record that GetFolder returning null for a case-differing spelling makes MySQL insert a second row where it used to reuse one — correct, and now matching SQLite, but a real behaviour change on a case-insensitive filesystem. Refs #488 #308 fix #491
313 lines
15 KiB
C#
313 lines
15 KiB
C#
using ErsatzTV.Core;
|
|
using ErsatzTV.Core.Domain;
|
|
using ErsatzTV.Infrastructure.Data;
|
|
using ErsatzTV.Infrastructure.Data.Repositories;
|
|
using ErsatzTV.Tests.Support;
|
|
using LanguageExt;
|
|
using Microsoft.EntityFrameworkCore;
|
|
using NSubstitute;
|
|
using NUnit.Framework;
|
|
using Shouldly;
|
|
using IFileSystem = System.IO.Abstractions.IFileSystem;
|
|
|
|
namespace ErsatzTV.Tests.Integration;
|
|
|
|
[TestFixture]
|
|
public class LibraryRepositoryTests
|
|
{
|
|
private InMemoryTvContext _db = null!;
|
|
private LibraryRepository _repository = null!;
|
|
|
|
[SetUp]
|
|
public async Task SetUp()
|
|
{
|
|
_db = await InMemoryTvContext.CreateAsync();
|
|
_repository = new LibraryRepository(Substitute.For<IFileSystem>(), _db.Factory);
|
|
}
|
|
|
|
[TearDown]
|
|
public async Task TearDown() => await _db.DisposeAsync();
|
|
|
|
// Regression for ersatztv#488: the Jellyfin (remote) sync path takes its LibraryPath straight off the
|
|
// JellyfinLibrary entity, so LibraryPath.LibraryFolders is never eager-loaded (null). GetOrAddFolder
|
|
// used to read that navigation collection directly and threw ArgumentNullException on the very first
|
|
// item of every Jellyfin music-video scan. The repository must resolve the folder from the database
|
|
// instead, so an unloaded collection is not a precondition.
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Create_Folder_When_LibraryFolders_Not_Loaded()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
|
|
// mimic the Jellyfin path: Paths is populated, but LibraryFolders was never included
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
LibraryFolder result =
|
|
await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
|
|
result.ShouldNotBeNull();
|
|
result.Id.ShouldBeGreaterThan(0);
|
|
result.Path.ShouldBe("/data/music/artist1");
|
|
result.LibraryPathId.ShouldBe(libraryPathId);
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
List<LibraryFolder> folders = await context.LibraryFolders
|
|
.Where(f => f.LibraryPathId == libraryPathId)
|
|
.ToListAsync();
|
|
folders.Count.ShouldBe(1);
|
|
folders[0].Path.ShouldBe("/data/music/artist1");
|
|
}
|
|
|
|
// Re-scanning must be idempotent: a second GetOrAddFolder for the same path returns the existing row
|
|
// rather than inserting a duplicate LibraryFolder (there is no unique constraint behind it).
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Be_Idempotent_On_Rescan()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
LibraryFolder first =
|
|
await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
LibraryFolder second =
|
|
await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
|
|
second.Id.ShouldBe(first.Id);
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
int count = await context.LibraryFolders.CountAsync(f => f.LibraryPathId == libraryPathId);
|
|
count.ShouldBe(1);
|
|
}
|
|
|
|
// Covers the maybeParentFolder = Some(...) branch: on a folder already in the db, the parent id is
|
|
// persisted through the raw Dapper UPDATE against the no-tracking entity (not change tracking), and the
|
|
// returned object reflects it. Guards the AsNoTracking + raw-UPDATE interaction the DB-lookup fix relies on.
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Persist_ParentId_On_Existing_Folder()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
LibraryFolder parent =
|
|
await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music");
|
|
// first pass creates the child with no parent
|
|
LibraryFolder child =
|
|
await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
child.ParentId.ShouldBeNull();
|
|
|
|
// second pass supplies the parent — the existing row must be updated, not duplicated
|
|
LibraryFolder updated =
|
|
await _repository.GetOrAddFolder(libraryPath, Option<int>.Some(parent.Id), "/data/music/artist1");
|
|
updated.Id.ShouldBe(child.Id);
|
|
updated.ParentId.ShouldBe(parent.Id);
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
LibraryFolder persisted = await context.LibraryFolders.SingleAsync(f => f.Id == child.Id);
|
|
persisted.ParentId.ShouldBe(parent.Id);
|
|
int count = await context.LibraryFolders.CountAsync(f => f.Path == "/data/music/artist1");
|
|
count.ShouldBe(1);
|
|
}
|
|
|
|
// ersatztv#491: the unique index is on (LibraryPathId, PathHash) because Path is unbounded, so every
|
|
// new row must carry the hash or the constraint is unenforceable for it.
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Populate_PathHash_On_New_Folder()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
LibraryFolder result = await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
|
|
result.PathHash.ShouldBe(PathUtils.GetPathHash("/data/music/artist1"));
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
LibraryFolder persisted = await context.LibraryFolders.SingleAsync(f => f.Id == result.Id);
|
|
persisted.PathHash.ShouldBe(PathUtils.GetPathHash("/data/music/artist1"));
|
|
}
|
|
|
|
// ersatztv#491: rows that predate the PathHash column are left null by the migration (nulls are
|
|
// distinct in a unique index, so the index applies cleanly); the first scan that touches one heals it.
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Heal_A_Legacy_Null_PathHash()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
int legacyId;
|
|
await using (TvContext seed = _db.CreateContext())
|
|
{
|
|
var legacy = new LibraryFolder
|
|
{
|
|
LibraryPathId = libraryPathId, Path = "/data/music/artist1", PathHash = null
|
|
};
|
|
await seed.LibraryFolders.AddAsync(legacy);
|
|
await seed.SaveChangesAsync();
|
|
legacyId = legacy.Id;
|
|
}
|
|
|
|
LibraryFolder result = await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
|
|
result.Id.ShouldBe(legacyId);
|
|
result.PathHash.ShouldBe(PathUtils.GetPathHash("/data/music/artist1"));
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
LibraryFolder persisted = await context.LibraryFolders.SingleAsync(f => f.Id == legacyId);
|
|
persisted.PathHash.ShouldBe(PathUtils.GetPathHash("/data/music/artist1"));
|
|
(await context.LibraryFolders.CountAsync(f => f.LibraryPathId == libraryPathId)).ShouldBe(1);
|
|
}
|
|
|
|
// ersatztv#491 / H1: when the lookup matches more than one row (legacy duplicates that predate the
|
|
// unique index, which is exactly the state the migration cleans up), it must resolve deterministically
|
|
// to the lowest Id. An unordered FirstOrDefault can return a different row as the plan changes — and
|
|
// adding the composite index alone can flip it — which would make the PathHash heal non-idempotent:
|
|
// each scan would heal a different row and the second would collide on (LibraryPathId, PathHash).
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Resolve_Legacy_Duplicates_Deterministically()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
int firstId;
|
|
await using (TvContext seed = _db.CreateContext())
|
|
{
|
|
var a = new LibraryFolder { LibraryPathId = libraryPathId, Path = "/data/music/artist1" };
|
|
var b = new LibraryFolder { LibraryPathId = libraryPathId, Path = "/data/music/artist1" };
|
|
await seed.LibraryFolders.AddRangeAsync(a, b);
|
|
await seed.SaveChangesAsync();
|
|
firstId = Math.Min(a.Id, b.Id);
|
|
}
|
|
|
|
// repeated calls must agree, and must agree with MIN(Id) — the same row the migration keeps
|
|
LibraryFolder first = await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
LibraryFolder second = await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
|
|
first.Id.ShouldBe(firstId);
|
|
second.Id.ShouldBe(firstId);
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
|
|
// exactly one of the two got the hash — the heal is idempotent, not alternating
|
|
(await context.LibraryFolders.CountAsync(
|
|
f => f.LibraryPathId == libraryPathId && f.PathHash != null)).ShouldBe(1);
|
|
LibraryFolder healed = await context.LibraryFolders.SingleAsync(f => f.Id == firstId);
|
|
healed.PathHash.ShouldBe(PathUtils.GetPathHash("/data/music/artist1"));
|
|
}
|
|
|
|
// ersatztv#491 / H1: on MySQL the lookup's SQL equality is case-INsensitive (longtext under the
|
|
// default collation), so it hands back BOTH "/x/Foo" and "/x/foo" for a "/x/foo" scan — verified on a
|
|
// real MySQL 8.4 server, where an unordered LIMIT 1 returns "/x/Foo". PathHash is a case-SENSITIVE
|
|
// hash, so identity has to be settled ordinally or the heal stamps the wrong path's hash onto a row.
|
|
//
|
|
// This pins that decision with no database at all, because no SQLite-backed test can: SQLite's `=` on
|
|
// TEXT is binary, so the case-differing candidate never reaches the in-memory step. Feeding the
|
|
// candidate list directly is the only way to exercise it in CI.
|
|
[Test]
|
|
public void ResolveExact_Should_Pick_The_Ordinal_Match_Not_A_Case_Variant()
|
|
{
|
|
var candidates = new List<LibraryFolder>
|
|
{
|
|
new() { Id = 10, Path = "/x/Foo" },
|
|
new() { Id = 11, Path = "/x/foo" }
|
|
};
|
|
|
|
LibraryRepository.ResolveExact(candidates, "/x/foo").Id.ShouldBe(11);
|
|
LibraryRepository.ResolveExact(candidates, "/x/Foo").Id.ShouldBe(10);
|
|
|
|
// a spelling that matches nothing ordinally is absent, not "close enough"
|
|
LibraryRepository.ResolveExact(candidates, "/x/FOO").ShouldBeNull();
|
|
LibraryRepository.ResolveExact([], "/x/foo").ShouldBeNull();
|
|
}
|
|
|
|
// Candidates arrive ordered by Id, and the first ordinal match wins — so true duplicates resolve to
|
|
// MIN(Id), the same row the #491 migration keeps.
|
|
[Test]
|
|
public void ResolveExact_Should_Prefer_The_First_Candidate_On_A_True_Duplicate()
|
|
{
|
|
var candidates = new List<LibraryFolder>
|
|
{
|
|
new() { Id = 3, Path = "/x/Foo" },
|
|
new() { Id = 7, Path = "/x/foo" },
|
|
new() { Id = 9, Path = "/x/foo" }
|
|
};
|
|
|
|
LibraryRepository.ResolveExact(candidates, "/x/foo").Id.ShouldBe(7);
|
|
}
|
|
|
|
// End-to-end companion to the ResolveExact tests above. On SQLite this passes on pre-fix code too (the
|
|
// SQL equality already excludes the case variant); its value is guarding the in-memory step against
|
|
// later being relaxed to OrdinalIgnoreCase, and covering the heal/no-duplicate behaviour around it.
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Not_Resolve_A_Folder_Differing_Only_In_Case()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
|
|
int upperId;
|
|
await using (TvContext seed = _db.CreateContext())
|
|
{
|
|
var upper = new LibraryFolder { LibraryPathId = libraryPathId, Path = "/data/music/Foo" };
|
|
var lower = new LibraryFolder { LibraryPathId = libraryPathId, Path = "/data/music/foo" };
|
|
await seed.LibraryFolders.AddRangeAsync(upper, lower);
|
|
await seed.SaveChangesAsync();
|
|
upperId = upper.Id;
|
|
}
|
|
|
|
LibraryFolder result = await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/foo");
|
|
|
|
result.Path.ShouldBe("/data/music/foo");
|
|
result.Id.ShouldNotBe(upperId);
|
|
result.PathHash.ShouldBe(PathUtils.GetPathHash("/data/music/foo"));
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
|
|
// the case-differing sibling must be untouched — no foreign hash stamped onto it
|
|
LibraryFolder upperPersisted = await context.LibraryFolders.SingleAsync(f => f.Id == upperId);
|
|
upperPersisted.Path.ShouldBe("/data/music/Foo");
|
|
upperPersisted.PathHash.ShouldBeNull();
|
|
|
|
// and no duplicate was inserted for either spelling
|
|
(await context.LibraryFolders.CountAsync(f => f.LibraryPathId == libraryPathId)).ShouldBe(2);
|
|
}
|
|
|
|
// The heal is opportunistic maintenance on a hot scan path: if some other row already owns
|
|
// (LibraryPathId, hash) — a legacy duplicate the migration's grouping could not see — it must leave
|
|
// the row unhealed rather than abort the scan.
|
|
[Test]
|
|
public async Task GetOrAddFolder_Should_Not_Fail_The_Scan_When_The_PathHash_Heal_Collides()
|
|
{
|
|
int libraryPathId = await SeedLibraryPath("/data/music");
|
|
var libraryPath = new LibraryPath { Id = libraryPathId, Path = "/data/music", LibraryFolders = null };
|
|
string hash = PathUtils.GetPathHash("/data/music/artist1");
|
|
|
|
int legacyId;
|
|
await using (TvContext seed = _db.CreateContext())
|
|
{
|
|
// a legacy row with a null hash, plus a squatter that already owns the hash it would heal to
|
|
var legacy = new LibraryFolder { LibraryPathId = libraryPathId, Path = "/data/music/artist1" };
|
|
var squatter = new LibraryFolder
|
|
{
|
|
LibraryPathId = libraryPathId, Path = "/data/music/squatter", PathHash = hash
|
|
};
|
|
await seed.LibraryFolders.AddRangeAsync(legacy, squatter);
|
|
await seed.SaveChangesAsync();
|
|
legacyId = legacy.Id;
|
|
}
|
|
|
|
// must not throw — the scan continues and simply returns the folder
|
|
LibraryFolder result = await _repository.GetOrAddFolder(libraryPath, Option<int>.None, "/data/music/artist1");
|
|
|
|
result.Id.ShouldBe(legacyId);
|
|
|
|
await using TvContext context = _db.CreateContext();
|
|
LibraryFolder persisted = await context.LibraryFolders.SingleAsync(f => f.Id == legacyId);
|
|
persisted.PathHash.ShouldBeNull(); // heal declined, not half-applied
|
|
}
|
|
|
|
private async Task<int> SeedLibraryPath(string path)
|
|
{
|
|
await using TvContext context = _db.CreateContext();
|
|
var libraryPath = new LibraryPath { Path = path };
|
|
await context.LibraryPaths.AddAsync(libraryPath);
|
|
await context.SaveChangesAsync();
|
|
return libraryPath.Id;
|
|
}
|
|
}
|