From fb055aa1fc44b06a7069327fa047cff933f75810 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 15 Sep 2026 11:13:45 -0400 Subject: Backport pull request #17835 from jellyfin/release-12.z Clean up invalid data before running migrations Original-merge: 75d7b2010ea2d75fdb7554c700c4e6981331b484 Merged-by: crobibero Backported-by: Cody Robibero --- ...20260825200000_ConsolidateLocalizedUserViews.cs | 28 +++ ...ProperParentChildRelationBaseItemWithCascade.cs | 34 ++-- .../20260113203012_ChangeOwnerIdToGuid.cs | 55 ++++++ ..._RemoveOrphanedUserPermissionsAndPreferences.cs | 4 +- .../ConsolidateLocalizedUserViewsTests.cs | 202 +++++++++++++++++++++ 5 files changed, 300 insertions(+), 23 deletions(-) create mode 100644 tests/Jellyfin.Server.Tests/Migrations/ConsolidateLocalizedUserViewsTests.cs diff --git a/Jellyfin.Server/Migrations/Routines/20260825200000_ConsolidateLocalizedUserViews.cs b/Jellyfin.Server/Migrations/Routines/20260825200000_ConsolidateLocalizedUserViews.cs index 3fc2387e09..8eefbdb63a 100644 --- a/Jellyfin.Server/Migrations/Routines/20260825200000_ConsolidateLocalizedUserViews.cs +++ b/Jellyfin.Server/Migrations/Routines/20260825200000_ConsolidateLocalizedUserViews.cs @@ -156,6 +156,7 @@ internal class ConsolidateLocalizedUserViews : IAsyncMigrationRoutine await MoveAncestorsAsync(dbContext, canonicalId, staleIds, cancellationToken).ConfigureAwait(false); await MoveUserSettingsAsync(dbContext, canonicalId, sourceId, staleIds, cancellationToken).ConfigureAwait(false); + await MoveRemainingReferencesAsync(dbContext, newParentId, staleIds, cancellationToken).ConfigureAwait(false); // Nothing points at them any more, and BaseItems cascades on ParentId, so this has to come last. await dbContext.BaseItems @@ -171,6 +172,31 @@ internal class ConsolidateLocalizedUserViews : IAsyncMigrationRoutine canonicalId); } + private static async Task MoveRemainingReferencesAsync( + JellyfinDbContext dbContext, + Guid? canonicalId, + IReadOnlyList staleIds, + CancellationToken cancellationToken) + { + await dbContext.BaseItems + .Where(e => e.OwnerId.HasValue) + .WhereOneOrMany(staleIds, e => e.OwnerId!.Value) + .ExecuteUpdateAsync(e => e.SetProperty(f => f.OwnerId, canonicalId), cancellationToken) + .ConfigureAwait(false); + + // Keyed by (ParentId, SortOrder), so these cannot be repointed onto the canonical view + // without risking a collision, and a view listing linked children is meaningless anyway. + await dbContext.LinkedChildren + .WhereOneOrMany(staleIds, e => e.ParentId) + .ExecuteDeleteAsync(cancellationToken) + .ConfigureAwait(false); + + await dbContext.LinkedChildren + .WhereOneOrMany(staleIds, e => e.ChildId) + .ExecuteDeleteAsync(cancellationToken) + .ConfigureAwait(false); + } + private async Task PickSourceAsync( JellyfinDbContext dbContext, IReadOnlyList stale, @@ -294,8 +320,10 @@ internal class ConsolidateLocalizedUserViews : IAsyncMigrationRoutine IReadOnlyList staleIds, CancellationToken cancellationToken) { + // Ancestry recorded against items that no longer exist is dead weight. var items = await dbContext.AncestorIds .WhereOneOrMany(staleIds, e => e.ParentItemId) + .Where(e => dbContext.BaseItems.Any(item => item.Id.Equals(e.ItemId))) .Select(e => e.ItemId) .Distinct() .ToListAsync(cancellationToken) diff --git a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20250913211637_AddProperParentChildRelationBaseItemWithCascade.cs b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20250913211637_AddProperParentChildRelationBaseItemWithCascade.cs index a7f5e369ab..156f553fb3 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20250913211637_AddProperParentChildRelationBaseItemWithCascade.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20250913211637_AddProperParentChildRelationBaseItemWithCascade.cs @@ -11,27 +11,19 @@ namespace Jellyfin.Server.Implementations.Migrations protected override void Up(MigrationBuilder migrationBuilder) { migrationBuilder.Sql(""" -DELETE FROM BaseItems - WHERE - ParentId IS NOT NULL - AND - NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId); -DELETE FROM BaseItems - WHERE - ParentId IS NOT NULL - AND - NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId); -DELETE FROM BaseItems - WHERE - ParentId IS NOT NULL - AND - NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId); -DELETE FROM BaseItems - WHERE - ParentId IS NOT NULL - AND - NOT EXISTS(SELECT 1 FROM BaseItems parent WHERE parent.Id = BaseItems.ParentId); -"""); + WITH RECURSIVE Orphan ("Id") AS ( + SELECT Child."Id" + FROM "BaseItems" AS Child + WHERE Child."ParentId" IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM "BaseItems" AS Parent WHERE Parent."Id" = Child."ParentId") + UNION + SELECT Descendant."Id" + FROM "BaseItems" AS Descendant + INNER JOIN Orphan ON Descendant."ParentId" = Orphan."Id" + ) + DELETE FROM "BaseItems" WHERE "Id" IN (SELECT "Id" FROM Orphan); + """); + migrationBuilder.AddForeignKey( name: "FK_BaseItems_BaseItems_ParentId", table: "BaseItems", diff --git a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260113203012_ChangeOwnerIdToGuid.cs b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260113203012_ChangeOwnerIdToGuid.cs index 4927b0e78d..379da0e9be 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260113203012_ChangeOwnerIdToGuid.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260113203012_ChangeOwnerIdToGuid.cs @@ -11,6 +11,61 @@ namespace Jellyfin.Database.Providers.Sqlite.Migrations /// protected override void Up(MigrationBuilder migrationBuilder) { + migrationBuilder.Sql( + """ + DROP TABLE IF EXISTS "OrphanedBaseItemIds"; + CREATE TEMPORARY TABLE "OrphanedBaseItemIds" ("Id" TEXT NOT NULL PRIMARY KEY); + + INSERT INTO "OrphanedBaseItemIds" ("Id") + WITH RECURSIVE Orphan ("Id") AS ( + SELECT Child."Id" + FROM "BaseItems" AS Child + WHERE Child."ParentId" IS NOT NULL + AND NOT EXISTS (SELECT 1 FROM "BaseItems" AS Parent WHERE Parent."Id" = Child."ParentId") + UNION + SELECT Descendant."Id" + FROM "BaseItems" AS Descendant + INNER JOIN Orphan ON Descendant."ParentId" = Orphan."Id" + ) + SELECT "Id" FROM Orphan; + + -- Keep the play state of the doomed items the way ItemPersistenceService does when it + -- deletes an item: reattach it to the placeholder item instead of letting the + -- FK_UserData_BaseItems_ItemId cascade wipe it. The placeholder can only hold one row + -- per (UserId, CustomDataKey), so resolve collisions before repointing anything. + DELETE FROM "UserData" + WHERE "ItemId" = '00000000-0000-0000-0000-000000000001' + AND EXISTS ( + SELECT 1 + FROM "UserData" AS Doomed + INNER JOIN "OrphanedBaseItemIds" AS Orphan ON Orphan."Id" = Doomed."ItemId" + WHERE Doomed."UserId" = "UserData"."UserId" + AND Doomed."CustomDataKey" = "UserData"."CustomDataKey"); + + DELETE FROM "UserData" + WHERE "ItemId" IN (SELECT "Id" FROM "OrphanedBaseItemIds") + AND "rowid" NOT IN ( + SELECT MIN("rowid") + FROM "UserData" + WHERE "ItemId" IN (SELECT "Id" FROM "OrphanedBaseItemIds") + GROUP BY "UserId", "CustomDataKey"); + + UPDATE "UserData" + SET "ItemId" = '00000000-0000-0000-0000-000000000001', + "RetentionDate" = datetime('now') + WHERE "ItemId" IN (SELECT "Id" FROM "OrphanedBaseItemIds"); + + -- FK_LinkedChildren_BaseItems_{ParentId,ChildId} are NO ACTION, so these rows have to + -- go by hand or the delete below fails on them. + DELETE FROM "LinkedChildren" + WHERE "ParentId" IN (SELECT "Id" FROM "OrphanedBaseItemIds") + OR "ChildId" IN (SELECT "Id" FROM "OrphanedBaseItemIds"); + + DELETE FROM "BaseItems" WHERE "Id" IN (SELECT "Id" FROM "OrphanedBaseItemIds"); + + DROP TABLE "OrphanedBaseItemIds"; + """); + // Normalize OwnerId to uppercase GUID format migrationBuilder.Sql( @"UPDATE BaseItems diff --git a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.cs b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.cs index 3d4cf90441..2530e84af6 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/Migrations/20260815063607_RemoveOrphanedUserPermissionsAndPreferences.cs @@ -11,8 +11,8 @@ namespace Jellyfin.Database.Providers.Sqlite.Migrations /// protected override void Up(MigrationBuilder migrationBuilder) { - migrationBuilder.Sql("DELETE FROM Permissions WHERE UserId IS NULL;"); - migrationBuilder.Sql("DELETE FROM Preferences WHERE UserId IS NULL;"); + migrationBuilder.Sql("DELETE FROM Permissions WHERE UserId IS NULL OR UserId NOT IN (SELECT Id FROM Users);"); + migrationBuilder.Sql("DELETE FROM Preferences WHERE UserId IS NULL OR UserId NOT IN (SELECT Id FROM Users);"); migrationBuilder.DropIndex( name: "IX_Preferences_UserId_Kind", diff --git a/tests/Jellyfin.Server.Tests/Migrations/ConsolidateLocalizedUserViewsTests.cs b/tests/Jellyfin.Server.Tests/Migrations/ConsolidateLocalizedUserViewsTests.cs new file mode 100644 index 0000000000..25430447d4 --- /dev/null +++ b/tests/Jellyfin.Server.Tests/Migrations/ConsolidateLocalizedUserViewsTests.cs @@ -0,0 +1,202 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.Data.Enums; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Locking; +using Jellyfin.Database.Providers.Sqlite; +using Jellyfin.Server.Migrations.Routines; +using Jellyfin.Server.ServerSetupApp; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Model.IO; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Tests.Migrations; + +/// +/// Covers the references a database carried up from 10.x can still hold against a view the migration +/// is about to drop. Only ParentId cascades; everything else here is a NO ACTION foreign key that +/// used to abort the migration, and with it the whole startup. +/// +public sealed class ConsolidateLocalizedUserViewsTests : IDisposable +{ + private const string MetadataPath = "/metadata"; + + private static readonly Guid _staleId = new("11111111-1111-1111-1111-111111111111"); + private static readonly Guid _canonicalId = new("22222222-2222-2222-2222-222222222222"); + + private readonly SqliteConnection _connection; + private readonly DbContextOptions _dbOptions; + + public ConsolidateLocalizedUserViewsTests() + { + _connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True"); + _connection.Open(); + + _dbOptions = new DbContextOptionsBuilder() + .UseSqlite(_connection) + .Options; + + using var context = CreateDbContext(); + context.Database.EnsureCreated(); + } + + private static CancellationToken Ct => TestContext.Current.CancellationToken; + + [Fact] + public async Task PerformAsync_ItemOwnedByStaleView_MovesItAndDropsTheView() + { + using (var context = CreateDbContext()) + { + context.BaseItems.Add(StaleView()); + context.BaseItems.Add(new BaseItemEntity { Id = Guid.NewGuid(), Type = "Trailer", OwnerId = _staleId }); + await context.SaveChangesAsync(Ct); + } + + await CreateMigration().PerformAsync(Ct); + + using (var context = CreateDbContext()) + { + Assert.Null(await context.BaseItems.FindAsync([_staleId], Ct)); + Assert.Equal(_canonicalId, (await context.BaseItems.SingleAsync(e => e.Type == "Trailer", Ct)).OwnerId); + } + } + + [Fact] + public async Task PerformAsync_StaleViewInLinkedChildren_DropsTheLinksAndTheView() + { + var movieId = Guid.NewGuid(); + + using (var context = CreateDbContext()) + { + context.BaseItems.Add(StaleView()); + context.BaseItems.Add(new BaseItemEntity { Id = movieId, Type = "Movie" }); + context.LinkedChildren.Add(new LinkedChildEntity { ParentId = _staleId, SortOrder = 0, ChildId = movieId, ChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType.Manual }); + context.LinkedChildren.Add(new LinkedChildEntity { ParentId = movieId, SortOrder = 0, ChildId = _staleId, ChildType = Jellyfin.Database.Implementations.Entities.LinkedChildType.Manual }); + await context.SaveChangesAsync(Ct); + } + + await CreateMigration().PerformAsync(Ct); + + using (var context = CreateDbContext()) + { + Assert.Null(await context.BaseItems.FindAsync([_staleId], Ct)); + Assert.Empty(context.LinkedChildren); + Assert.NotNull(await context.BaseItems.FindAsync([movieId], Ct)); + } + } + + [Fact] + public async Task PerformAsync_OrphanedAncestry_IsNotResurrectedUnderTheCanonicalView() + { + var childId = Guid.NewGuid(); + var orphanId = Guid.NewGuid(); + + using (var context = CreateDbContext()) + { + context.BaseItems.Add(StaleView()); + context.BaseItems.Add(new BaseItemEntity { Id = childId, Type = "Movie", ParentId = _staleId }); + await context.SaveChangesAsync(Ct); + + context.AncestorIds.Add(new AncestorId { ItemId = childId, ParentItemId = _staleId, Item = null!, ParentItem = null! }); + await context.SaveChangesAsync(Ct); + + // Written while foreign keys went unenforced: the item behind it is long gone. + await context.Database.ExecuteSqlRawAsync("PRAGMA foreign_keys = OFF", Ct); + context.AncestorIds.Add(new AncestorId { ItemId = orphanId, ParentItemId = _staleId, Item = null!, ParentItem = null! }); + await context.SaveChangesAsync(Ct); + await context.Database.ExecuteSqlRawAsync("PRAGMA foreign_keys = ON", Ct); + } + + await CreateMigration().PerformAsync(Ct); + + using (var context = CreateDbContext()) + { + Assert.Null(await context.BaseItems.FindAsync([_staleId], Ct)); + Assert.Equal(_canonicalId, (await context.BaseItems.SingleAsync(e => e.Id.Equals(childId), Ct)).ParentId); + var ancestors = await context.AncestorIds.ToListAsync(Ct); + Assert.Equal(new[] { childId }, ancestors.Select(e => e.ItemId)); + Assert.Equal(_canonicalId, ancestors[0].ParentItemId); + } + } + + public void Dispose() + { + _connection.Dispose(); + GC.SuppressFinalize(this); + } + + private static BaseItemEntity StaleView() => new() + { + Id = _staleId, + Type = "MediaBrowser.Controller.Entities.UserView", + Path = Path.Combine(MetadataPath, "views", "livetv") + }; + + private JellyfinDbContext CreateDbContext() => new( + _dbOptions, + NullLogger.Instance, + new SqliteDatabaseProvider(new Mock().Object, NullLogger.Instance), + new NoLockBehavior(NullLogger.Instance)); + + private ConsolidateLocalizedUserViews CreateMigration() + { + var view = new UserView + { + Id = _staleId, + Path = Path.Combine(MetadataPath, "views", "livetv"), + Name = "Live TV", + ViewType = CollectionType.livetv + }; + + var applicationPaths = new Mock(); + applicationPaths.Setup(e => e.InternalMetadataPath).Returns(MetadataPath); + + var configurationManager = new Mock(); + configurationManager.Setup(e => e.ApplicationPaths).Returns(applicationPaths.Object); + + var fileSystem = new Mock(); + fileSystem.Setup(e => e.GetValidFilename(It.IsAny())).Returns((string name) => name); + + var libraryManager = new Mock(); + libraryManager.Setup(e => e.GetItemList(It.IsAny())) + .Returns(new List { view }); + libraryManager.Setup(e => e.GetNewItemId(It.IsAny(), It.IsAny())) + .Returns(_canonicalId); + libraryManager.Setup(e => e.CreateItem(It.IsAny(), It.IsAny())) + .Callback((BaseItem item, BaseItem? parent) => + { + using var context = CreateDbContext(); + context.BaseItems.Add(new BaseItemEntity + { + Id = item.Id, + Type = item.GetType().FullName!, + Path = item.Path + }); + context.SaveChanges(); + }); + + var factory = new Mock>(); + factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + factory.Setup(f => f.CreateDbContextAsync(It.IsAny())).ReturnsAsync(CreateDbContext); + + return new ConsolidateLocalizedUserViews( + new StartupLogger(NullLogger.Instance), + libraryManager.Object, + configurationManager.Object, + fileSystem.Object, + factory.Object); + } +} -- cgit v1.2.3