Cross-family review of 1b4dd6d6 found that utf8mb4_bin - chosen to keep the dedupe case-exact - is a PAD SPACE collation, so trailing spaces are insignificant under it. Verified on MySQL 8.4: '/media/Foo' = '/media/Foo ' is TRUE, while case correctly compares unequal. Two distinct legal directories therefore grouped together and the second was DELETED irreversibly, even though PathUtils.GetPathHash hashes them differently and the unique index about to be created would have accepted both. The dedupe destroyed data the constraint never required it to destroy. Group and join on CONVERT(Path USING binary) instead - NO PAD and byte-exact, matching the hash. utf8mb4_0900_bin is also NO PAD but carries a server-version floor. This is the only path comparison in either migration (every other predicate keys off an integer id), so there is no mix of padded and unpadded comparisons across the keeper-selection, repoint and delete steps. SQLite's = on TEXT is byte-exact with no padding, so that migration was already correct - which is exactly why a SQLite-only test could not see the divergence. The two providers are now semantically equivalent, and the dedupe fixture is shared: same rows, same expected survivors (1,4,5,6,7,9,10), asserted by the SQLite test and reproduced by hand on MySQL 8.4. Runtime was never affected, and this is now stated and tested rather than assumed: GetFolder's SQL equality is a superset narrowing (both collation quirks make it more permissive, never less, so it cannot miss a byte-exact match) and ResolveExact settles identity with StringComparison.Ordinal, which compares length first. Added ResolveExact coverage for the trailing-space axis. Refs #488 #308 fix #491
257 lines
13 KiB
C#
257 lines
13 KiB
C#
using Dapper;
|
|
using ErsatzTV.Infrastructure;
|
|
using ErsatzTV.Infrastructure.Data;
|
|
using ErsatzTV.Infrastructure.Sqlite.Data;
|
|
using Microsoft.EntityFrameworkCore;
|
|
using Microsoft.Extensions.Logging.Abstractions;
|
|
using NUnit.Framework;
|
|
using Shouldly;
|
|
|
|
namespace ErsatzTV.Tests.Integration;
|
|
|
|
/// <summary>
|
|
/// ersatztv#491: the unique index on <c>LibraryFolder(LibraryPathId, PathHash)</c> ships with an
|
|
/// audit/cleanup of pre-existing duplicate rows (reachable before #488, when the folder lookup read a
|
|
/// scan-start in-memory snapshot and could not see a folder created earlier in the same scan). This
|
|
/// drives the REAL Sqlite migration against a database seeded at the previous migration, so the
|
|
/// cleanup SQL is exercised rather than restated.
|
|
/// </summary>
|
|
[TestFixture]
|
|
public class LibraryFolderDedupeMigrationTests
|
|
{
|
|
// the migration immediately preceding Add_LibraryFolder_PathHash_UniqueIndex
|
|
private const string PreviousMigration = "Add_Channel_Origin";
|
|
|
|
private string _databasePath = null!;
|
|
|
|
// Seeding writes deliberately partial object graphs (a LibraryPath with no Library, a MediaFile with
|
|
// no MediaVersion), so it needs foreign keys OFF — as every other harness in this suite does.
|
|
private DbContextOptions<TvContext> _seedOptions = null!;
|
|
|
|
// The migration itself runs with foreign keys ON, matching production (`Startup.cs` builds the SQLite
|
|
// connection string with `foreign keys=true`). This matters: the single most dangerous statement in
|
|
// the #491 migration is `DELETE FROM LibraryFolder` against two Restrict foreign keys
|
|
// (MediaFile.LibraryFolderId, LibraryFolder.ParentId). With enforcement off, a wrong repoint order
|
|
// would still pass; with it on, the delete fails loudly.
|
|
private DbContextOptions<TvContext> _migrateOptions = null!;
|
|
|
|
[SetUp]
|
|
public void SetUp()
|
|
{
|
|
TvContext.IsSqlite = true;
|
|
TvContext.IsUniqueConstraintViolation = SqliteErrorClassifier.IsUniqueConstraintViolation;
|
|
|
|
_databasePath = Path.Combine(Path.GetTempPath(), $"etv491-{Guid.NewGuid():N}.sqlite3");
|
|
_seedOptions = BuildOptions(foreignKeys: false);
|
|
_migrateOptions = BuildOptions(foreignKeys: true);
|
|
}
|
|
|
|
private DbContextOptions<TvContext> BuildOptions(bool foreignKeys) =>
|
|
new DbContextOptionsBuilder<TvContext>()
|
|
.UseSqlite(
|
|
$"Data Source={_databasePath};Foreign Keys={foreignKeys}",
|
|
o => o.MigrationsAssembly("ErsatzTV.Infrastructure.Sqlite"))
|
|
.Options;
|
|
|
|
[TearDown]
|
|
public void TearDown()
|
|
{
|
|
Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools();
|
|
foreach (string path in new[] { _databasePath, $"{_databasePath}-wal", $"{_databasePath}-shm" })
|
|
{
|
|
if (File.Exists(path))
|
|
{
|
|
File.Delete(path);
|
|
}
|
|
}
|
|
}
|
|
|
|
[Test]
|
|
public async Task Migration_Collapses_Duplicate_Folders_And_Repoints_Their_Dependents()
|
|
{
|
|
await using (TvContext context = SeedContext())
|
|
{
|
|
await context.Database.MigrateAsync(PreviousMigration);
|
|
|
|
// one library path with the SAME folder recorded three times (ids 1, 2, 3) plus an unrelated
|
|
// folder (id 4) and a child parented on one of the duplicates (id 5)
|
|
await context.Connection.ExecuteAsync(
|
|
"INSERT INTO LibraryPath (Id, LibraryId, Path) VALUES (1, 1, '/data/music')");
|
|
await context.Connection.ExecuteAsync(
|
|
"""
|
|
INSERT INTO LibraryFolder (Id, LibraryPathId, Path, ParentId, Etag) VALUES
|
|
(1, 1, '/data/music/artist1', NULL, 'etag-keeper'),
|
|
(2, 1, '/data/music/artist1', NULL, 'etag-dupe-a'),
|
|
(3, 1, '/data/music/artist1', NULL, 'etag-dupe-b'),
|
|
(4, 1, '/data/music/artist2', NULL, NULL),
|
|
(5, 1, '/data/music/artist1/album', 3, NULL),
|
|
-- a case-differing sibling, legal on a case-sensitive filesystem: must SURVIVE
|
|
(6, 1, '/data/music/ARTIST2', NULL, NULL),
|
|
-- duplicate 3 is parented on duplicate 2: repointing both would make the survivor
|
|
-- its own parent, the cycle the ParentId null-out guards
|
|
(7, 1, '/data/music/artist3', 8, NULL),
|
|
(8, 1, '/data/music/artist3', NULL, NULL),
|
|
-- a sibling differing only by a TRAILING SPACE. Two distinct legal directories on
|
|
-- Linux, and PathUtils.GetPathHash hashes them differently, so the unique index would
|
|
-- accept both — the dedupe must not collapse them. MySql's string comparison is PAD
|
|
-- SPACE (verified on 8.4, for utf8mb4_bin as well as the ci default), so grouping
|
|
-- under any collation rather than binary would delete row 10 irreversibly.
|
|
(9, 1, '/data/music/pad', NULL, NULL),
|
|
(10, 1, '/data/music/pad ', NULL, NULL)
|
|
""");
|
|
|
|
// a media file on each duplicate, and an image-folder-duration on a duplicate only
|
|
await context.Connection.ExecuteAsync(
|
|
"""
|
|
INSERT INTO MediaFile (Id, Path, PathHash, MediaVersionId, LibraryFolderId) VALUES
|
|
(1, '/data/music/artist1/a.mkv', 'hash-a', 1, 1),
|
|
(2, '/data/music/artist1/b.mkv', 'hash-b', 2, 2),
|
|
(3, '/data/music/artist1/c.mkv', 'hash-c', 3, 3),
|
|
-- dependents of the trailing-space pair: must stay attached to their OWN folder
|
|
(4, '/data/music/pad/d.mkv', 'hash-d', 4, 9),
|
|
(5, '/data/music/pad /e.mkv', 'hash-e', 5, 10)
|
|
""");
|
|
await context.Connection.ExecuteAsync(
|
|
"""
|
|
INSERT INTO ImageFolderDuration (Id, LibraryFolderId, DurationSeconds) VALUES
|
|
(1, 2, 30.0),
|
|
(2, 3, 45.0)
|
|
""");
|
|
}
|
|
|
|
await using (TvContext context = MigrateContext())
|
|
{
|
|
await AssertForeignKeysEnforced(context);
|
|
await context.Database.MigrateAsync();
|
|
}
|
|
|
|
await using (TvContext context = MigrateContext())
|
|
{
|
|
// the duplicates are gone; the lowest id survives
|
|
List<int> folderIds =
|
|
(await context.Connection.QueryAsync<int>(
|
|
"SELECT Id FROM LibraryFolder WHERE Path = '/data/music/artist1'")).ToList();
|
|
folderIds.ShouldBe([1]);
|
|
|
|
// survivors: keeper 1, artist2 (4), the child (5), the case-differing sibling (6), artist3's
|
|
// keeper (7), and BOTH halves of the trailing-space pair (9, 10). Losers 2, 3 and 8 are gone.
|
|
List<int> survivors =
|
|
(await context.Connection.QueryAsync<int>(
|
|
"SELECT Id FROM LibraryFolder ORDER BY Id")).ToList();
|
|
survivors.ShouldBe([1, 4, 5, 6, 7, 9, 10]);
|
|
|
|
// a folder differing only in case is NOT a duplicate — it must survive untouched
|
|
(await context.Connection.ExecuteScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM LibraryFolder WHERE Path = '/data/music/ARTIST2'")).ShouldBe(1);
|
|
|
|
// nor is one differing only by a trailing space, and each keeps its OWN dependents
|
|
List<string> padded =
|
|
(await context.Connection.QueryAsync<string>(
|
|
"SELECT Id || '=[' || Path || ']' FROM LibraryFolder WHERE Id IN (9, 10) ORDER BY Id"))
|
|
.ToList();
|
|
padded.ShouldBe(["9=[/data/music/pad]", "10=[/data/music/pad ]"]);
|
|
|
|
List<int> padMediaFolderIds =
|
|
(await context.Connection.QueryAsync<int>(
|
|
"SELECT LibraryFolderId FROM MediaFile WHERE Id IN (4, 5) ORDER BY Id")).ToList();
|
|
padMediaFolderIds.ShouldBe([9, 10]);
|
|
|
|
// the keeper's etag is cleared: which duplicate the scanner was writing to was arbitrary, so
|
|
// MIN(Id)'s etag could suppress the rescan that repairs the collapsed folder
|
|
(await context.Connection.ExecuteScalarAsync<string>(
|
|
"SELECT Etag FROM LibraryFolder WHERE Id = 1")).ShouldBeNull();
|
|
|
|
// the folder parented on its own duplicate did not become its own parent
|
|
(await context.Connection.ExecuteScalarAsync<int?>(
|
|
"SELECT ParentId FROM LibraryFolder WHERE Id = 7")).ShouldBeNull();
|
|
|
|
// every media file follows the keeper — nothing orphaned, nothing deleted
|
|
List<int> mediaFolderIds =
|
|
(await context.Connection.QueryAsync<int>(
|
|
"SELECT LibraryFolderId FROM MediaFile WHERE Id IN (1, 2, 3) ORDER BY Id")).ToList();
|
|
mediaFolderIds.ShouldBe([1, 1, 1]);
|
|
|
|
// the child folder is reparented off the deleted duplicate onto the keeper
|
|
(await context.Connection.ExecuteScalarAsync<int>(
|
|
"SELECT ParentId FROM LibraryFolder WHERE Id = 5")).ShouldBe(1);
|
|
|
|
// the keeper had no ImageFolderDuration, so exactly one duplicate's setting is promoted to it
|
|
// (the lowest id) and the rest are dropped — the 1:1 unique index cannot hold both
|
|
List<string> durations =
|
|
(await context.Connection.QueryAsync<string>(
|
|
"SELECT Id || ':' || LibraryFolderId FROM ImageFolderDuration ORDER BY Id")).ToList();
|
|
durations.ShouldBe(["1:1"]);
|
|
|
|
// the helper tables the cleanup used are not left behind
|
|
(await context.Connection.ExecuteScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name LIKE '__LibraryFolderDedupe%'"))
|
|
.ShouldBe(0);
|
|
|
|
// and the unique index is now in place and enforcing
|
|
(await context.Connection.ExecuteScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sqlite_master WHERE type = 'index' AND name = 'IX_LibraryFolder_LibraryPathId_PathHash'"))
|
|
.ShouldBe(1);
|
|
}
|
|
}
|
|
|
|
[Test]
|
|
public async Task Migration_Leaves_A_Database_Without_Duplicates_Alone()
|
|
{
|
|
// positive control: the cleanup must not touch rows that were already unique
|
|
await using (TvContext context = SeedContext())
|
|
{
|
|
await context.Database.MigrateAsync(PreviousMigration);
|
|
await context.Connection.ExecuteAsync(
|
|
"INSERT INTO LibraryPath (Id, LibraryId, Path) VALUES (1, 1, '/data/music')");
|
|
await context.Connection.ExecuteAsync(
|
|
"""
|
|
INSERT INTO LibraryFolder (Id, LibraryPathId, Path, ParentId, Etag) VALUES
|
|
(1, 1, '/data/music/artist1', NULL, 'etag-1'),
|
|
(2, 1, '/data/music/artist2', 1, 'etag-2')
|
|
""");
|
|
await context.Connection.ExecuteAsync(
|
|
"INSERT INTO ImageFolderDuration (Id, LibraryFolderId, DurationSeconds) VALUES (1, 2, 30.0)");
|
|
}
|
|
|
|
await using (TvContext context = MigrateContext())
|
|
{
|
|
await AssertForeignKeysEnforced(context);
|
|
await context.Database.MigrateAsync();
|
|
}
|
|
|
|
await using (TvContext context = MigrateContext())
|
|
{
|
|
(await context.Connection.ExecuteScalarAsync<int>("SELECT COUNT(*) FROM LibraryFolder")).ShouldBe(2);
|
|
(await context.Connection.ExecuteScalarAsync<int>("SELECT ParentId FROM LibraryFolder WHERE Id = 2"))
|
|
.ShouldBe(1);
|
|
(await context.Connection.ExecuteScalarAsync<int>(
|
|
"SELECT LibraryFolderId FROM ImageFolderDuration WHERE Id = 1")).ShouldBe(2);
|
|
|
|
// existing rows keep a null hash — the index applies because nulls are distinct
|
|
(await context.Connection.ExecuteScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM LibraryFolder WHERE PathHash IS NULL")).ShouldBe(2);
|
|
}
|
|
}
|
|
|
|
// The `Foreign Keys=True` keyword above is the whole point of splitting the contexts, so prove it took
|
|
// effect rather than trusting the connection string: a typo or a pragma reset would silently revert
|
|
// this test to its weaker "enforcement off" form and still pass every assertion.
|
|
private static async Task AssertForeignKeysEnforced(TvContext context)
|
|
{
|
|
(await context.Connection.ExecuteScalarAsync<long>("PRAGMA foreign_keys")).ShouldBe(
|
|
1,
|
|
"the migration must run with foreign keys ENFORCED — otherwise DELETE FROM LibraryFolder is "
|
|
+ "not actually tested against the Restrict constraints it must not violate");
|
|
}
|
|
|
|
private TvContext SeedContext() => Create(_seedOptions);
|
|
|
|
private TvContext MigrateContext() => Create(_migrateOptions);
|
|
|
|
private static TvContext Create(DbContextOptions<TvContext> options) =>
|
|
new(
|
|
options,
|
|
NullLoggerFactory.Instance,
|
|
new SlowQueryInterceptor(NullLogger<SlowQueryInterceptor>.Instance));
|
|
}
|