Project the lowered person credit values once when updating people
This commit is contained in:
Shadowghost
@@ -117,13 +117,17 @@ public class PeopleRepository(IDbContextFactory<JellyfinDbContext> dbProvider, I
|
||||
person.Role = person.Role?.Trim() ?? string.Empty;
|
||||
}
|
||||
|
||||
// Project the values every comparison below needs once, so neither the case folding nor the
|
||||
// enum formatting is repeated per candidate.
|
||||
var credits = people.Select(e => (Person: e, LoweredName: e.Name.ToLowerInvariant(), PersonType: e.Type.ToString(), LoweredRole: e.Role.ToLowerInvariant()));
|
||||
|
||||
// multiple metadata providers can provide the _same_ credit; dedupe case-insensitively.
|
||||
// The role is part of the key because one person can hold several credits of the same type
|
||||
// on an item, e.g. a Writer credited for both the Novel and the Screenplay.
|
||||
people = people.DistinctBy(e => e.Name.ToLowerInvariant() + "-" + e.Type + "-" + e.Role.ToLowerInvariant()).ToArray();
|
||||
var distinctCredits = credits.DistinctBy(e => (e.LoweredName, e.PersonType, e.LoweredRole)).ToArray();
|
||||
|
||||
var distinctPersons = people.DistinctBy(e => e.Name.ToLowerInvariant() + "-" + e.Type).ToArray();
|
||||
var personKeys = distinctPersons.Select(e => e.Name.ToLowerInvariant() + "-" + e.Type).ToArray();
|
||||
var distinctPersons = distinctCredits.DistinctBy(e => (e.LoweredName, e.PersonType)).ToArray();
|
||||
var personKeys = distinctPersons.Select(e => e.LoweredName + "-" + e.PersonType).ToArray();
|
||||
|
||||
using var context = _dbProvider.CreateDbContext();
|
||||
using var transaction = context.Database.BeginTransaction();
|
||||
@@ -136,24 +140,44 @@ public class PeopleRepository(IDbContextFactory<JellyfinDbContext> dbProvider, I
|
||||
.Select(f => f.item)
|
||||
.ToArray();
|
||||
|
||||
var existingPersonKeys = existingPersons.Select(e => (e.Name.ToLowerInvariant(), e.PersonType ?? string.Empty)).ToHashSet();
|
||||
|
||||
var toAdd = distinctPersons
|
||||
.Where(e => !existingPersons.Any(f => string.Equals(f.Name, e.Name, StringComparison.OrdinalIgnoreCase) && f.PersonType == e.Type.ToString()))
|
||||
.Select(Map)
|
||||
.Where(e => !existingPersonKeys.Contains((e.LoweredName, e.PersonType)))
|
||||
.Select(e => Map(e.Person))
|
||||
.ToArray();
|
||||
context.Peoples.AddRange(toAdd);
|
||||
context.SaveChanges();
|
||||
|
||||
var personsEntities = toAdd.Concat(existingPersons).ToArray();
|
||||
// The Peoples table can hold case-only duplicates, so keep the first match per key just as
|
||||
// the previous First() lookup did.
|
||||
var personsEntities = new Dictionary<(string LoweredName, string PersonType), People>();
|
||||
foreach (var entity in toAdd.Concat(existingPersons))
|
||||
{
|
||||
personsEntities.TryAdd((entity.Name.ToLowerInvariant(), entity.PersonType ?? string.Empty), entity);
|
||||
}
|
||||
|
||||
var existingMaps = context.PeopleBaseItemMap.Include(e => e.People).Where(e => e.ItemId == itemId).ToList();
|
||||
var existingMapsByCredit = new Dictionary<(string LoweredName, string PersonType, string LoweredRole), PeopleBaseItemMap>();
|
||||
foreach (var map in existingMaps)
|
||||
{
|
||||
existingMapsByCredit.TryAdd((map.People.Name.ToLowerInvariant(), map.People.PersonType ?? string.Empty, map.Role?.ToLowerInvariant() ?? string.Empty), map);
|
||||
}
|
||||
|
||||
var listOrder = 0;
|
||||
|
||||
foreach (var person in people)
|
||||
foreach (var credit in distinctCredits)
|
||||
{
|
||||
var entityPerson = personsEntities.First(e => string.Equals(e.Name, person.Name, StringComparison.OrdinalIgnoreCase) && e.PersonType == person.Type.ToString());
|
||||
var existingMap = existingMaps.FirstOrDefault(e => string.Equals(e.People.Name, person.Name, StringComparison.OrdinalIgnoreCase) && e.People.PersonType == person.Type.ToString() && e.Role == person.Role);
|
||||
if (existingMap is null)
|
||||
var entityPerson = personsEntities[(credit.LoweredName, credit.PersonType)];
|
||||
if (existingMapsByCredit.TryGetValue((credit.LoweredName, credit.PersonType, credit.LoweredRole), out var existingMap))
|
||||
{
|
||||
// Update the order for existing mappings
|
||||
existingMap.ListOrder = listOrder;
|
||||
existingMap.SortOrder = credit.Person.SortOrder;
|
||||
// person mapping already exists so remove from list
|
||||
existingMaps.Remove(existingMap);
|
||||
}
|
||||
else
|
||||
{
|
||||
context.PeopleBaseItemMap.Add(new PeopleBaseItemMap()
|
||||
{
|
||||
@@ -162,18 +186,10 @@ public class PeopleRepository(IDbContextFactory<JellyfinDbContext> dbProvider, I
|
||||
People = null!,
|
||||
PeopleId = entityPerson.Id,
|
||||
ListOrder = listOrder,
|
||||
SortOrder = person.SortOrder,
|
||||
Role = person.Role
|
||||
SortOrder = credit.Person.SortOrder,
|
||||
Role = credit.Person.Role
|
||||
});
|
||||
}
|
||||
else
|
||||
{
|
||||
// Update the order for existing mappings
|
||||
existingMap.ListOrder = listOrder;
|
||||
existingMap.SortOrder = person.SortOrder;
|
||||
// person mapping already exists so remove from list
|
||||
existingMaps.Remove(existingMap);
|
||||
}
|
||||
|
||||
listOrder++;
|
||||
}
|
||||
|
||||
+186
@@ -0,0 +1,186 @@
|
||||
using System;
|
||||
using System.Linq;
|
||||
using Emby.Server.Implementations.Data;
|
||||
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.Implementations.Item;
|
||||
using MediaBrowser.Controller.Entities;
|
||||
using MediaBrowser.Controller.Persistence;
|
||||
using Microsoft.Data.Sqlite;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Moq;
|
||||
using Xunit;
|
||||
using BaseItemKind = Jellyfin.Data.Enums.BaseItemKind;
|
||||
|
||||
namespace Jellyfin.Server.Implementations.Tests.Item;
|
||||
|
||||
public sealed class PeopleRepositoryUpdatePeopleTests : IDisposable
|
||||
{
|
||||
private static readonly Guid _itemId = Guid.Parse("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa");
|
||||
|
||||
private readonly SqliteConnection _connection;
|
||||
private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
|
||||
private readonly PeopleRepository _repository;
|
||||
|
||||
public PeopleRepositoryUpdatePeopleTests()
|
||||
{
|
||||
_connection = new SqliteConnection("Data Source=:memory:");
|
||||
_connection.Open();
|
||||
|
||||
_dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
|
||||
.UseSqlite(_connection)
|
||||
.Options;
|
||||
|
||||
var itemTypeLookup = new ItemTypeLookup();
|
||||
|
||||
using (var ctx = CreateDbContext())
|
||||
{
|
||||
ctx.Database.EnsureCreated();
|
||||
ctx.BaseItems.Add(new BaseItemEntity
|
||||
{
|
||||
Id = _itemId,
|
||||
Type = itemTypeLookup.BaseItemKindNames[BaseItemKind.Movie],
|
||||
Name = "Movie",
|
||||
MediaType = "Video",
|
||||
IsMovie = true,
|
||||
IsFolder = false,
|
||||
IsVirtualItem = false
|
||||
});
|
||||
ctx.SaveChanges();
|
||||
}
|
||||
|
||||
var factory = new Mock<IDbContextFactory<JellyfinDbContext>>();
|
||||
factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext);
|
||||
|
||||
_repository = new PeopleRepository(
|
||||
factory.Object,
|
||||
itemTypeLookup,
|
||||
new Mock<IItemQueryHelpers>().Object);
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
_connection.Dispose();
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdatePeople_SamePersonAndTypeWithDifferentRoles_KeepsEveryCredit()
|
||||
{
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person A", PersonKind.Writer, "Novel"),
|
||||
CreatePerson("Person A", PersonKind.Writer, "Screenplay")
|
||||
]);
|
||||
|
||||
using var ctx = CreateDbContext();
|
||||
Assert.Single(ctx.Peoples);
|
||||
Assert.Equal(
|
||||
["Novel", "Screenplay"],
|
||||
ctx.PeopleBaseItemMap.OrderBy(e => e.ListOrder).Select(e => e.Role ?? string.Empty).ToArray());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdatePeople_CreditsDifferingOnlyInCase_AreDeduped()
|
||||
{
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person A", PersonKind.Actor, "Hero"),
|
||||
CreatePerson("person a", PersonKind.Actor, "hero")
|
||||
]);
|
||||
|
||||
using var ctx = CreateDbContext();
|
||||
Assert.Single(ctx.Peoples);
|
||||
var map = Assert.Single(ctx.PeopleBaseItemMap);
|
||||
Assert.Equal("Hero", map.Role);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdatePeople_SamePersonAsDifferentTypes_CreatesOnePersonPerType()
|
||||
{
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person A", PersonKind.Actor, "Hero"),
|
||||
CreatePerson("Person A", PersonKind.Director, string.Empty)
|
||||
]);
|
||||
|
||||
using var ctx = CreateDbContext();
|
||||
Assert.Equal(2, ctx.Peoples.Count());
|
||||
Assert.Equal(2, ctx.PeopleBaseItemMap.Count());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdatePeople_RepeatedUpdate_ReusesMappingsAndRefreshesOrder()
|
||||
{
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person A", PersonKind.Actor, "Hero"),
|
||||
CreatePerson("Person B", PersonKind.Actor, "Sidekick")
|
||||
]);
|
||||
|
||||
Guid[] peopleIdsBefore;
|
||||
using (var ctx = CreateDbContext())
|
||||
{
|
||||
peopleIdsBefore = ctx.Peoples.Select(e => e.Id).OrderBy(e => e).ToArray();
|
||||
}
|
||||
|
||||
// Reversed order, so the list order of both mappings has to be rewritten.
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person B", PersonKind.Actor, "Sidekick"),
|
||||
CreatePerson("Person A", PersonKind.Actor, "Hero")
|
||||
]);
|
||||
|
||||
using var after = CreateDbContext();
|
||||
Assert.Equal(peopleIdsBefore, after.Peoples.Select(e => e.Id).OrderBy(e => e).ToArray());
|
||||
Assert.Equal(
|
||||
["Sidekick", "Hero"],
|
||||
after.PeopleBaseItemMap.OrderBy(e => e.ListOrder).Select(e => e.Role ?? string.Empty).ToArray());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdatePeople_CreditRemoved_DropsOnlyThatMapping()
|
||||
{
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person A", PersonKind.Writer, "Novel"),
|
||||
CreatePerson("Person A", PersonKind.Writer, "Screenplay")
|
||||
]);
|
||||
|
||||
_repository.UpdatePeople(_itemId, [
|
||||
CreatePerson("Person A", PersonKind.Writer, "Novel")
|
||||
]);
|
||||
|
||||
using var ctx = CreateDbContext();
|
||||
var map = Assert.Single(ctx.PeopleBaseItemMap);
|
||||
Assert.Equal("Novel", map.Role);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UpdatePeople_RoleCaseChanged_KeepsExistingMapping()
|
||||
{
|
||||
_repository.UpdatePeople(_itemId, [CreatePerson("Person A", PersonKind.Actor, "Hero")]);
|
||||
|
||||
_repository.UpdatePeople(_itemId, [CreatePerson("Person A", PersonKind.Actor, "HERO")]);
|
||||
|
||||
using var ctx = CreateDbContext();
|
||||
var map = Assert.Single(ctx.PeopleBaseItemMap);
|
||||
Assert.Equal("Hero", map.Role);
|
||||
}
|
||||
|
||||
private static PersonInfo CreatePerson(string name, PersonKind type, string role)
|
||||
{
|
||||
return new PersonInfo
|
||||
{
|
||||
Name = name,
|
||||
Type = type,
|
||||
Role = role
|
||||
};
|
||||
}
|
||||
|
||||
private JellyfinDbContext CreateDbContext()
|
||||
{
|
||||
return new JellyfinDbContext(
|
||||
_dbOptions,
|
||||
NullLogger<JellyfinDbContext>.Instance,
|
||||
new SqliteDatabaseProvider(null!, NullLogger<SqliteDatabaseProvider>.Instance),
|
||||
new NoLockBehavior(NullLogger<NoLockBehavior>.Instance));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user