Backport pull request #18004 from jellyfin/release-12.z

Avoid full people scans and writes for unchanged credits

Original-merge: 63c81a9975c339a0757142cb69a318b50b85d216

Merged-by: crobibero <cody@robibe.ro>

Backported-by: Cody Robibero <cody@robibe.ro>

2 files changed, 141 insertions, 1 deletions

diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleUpdateQueryTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleUpdateQueryTests.cs
new file mode 100644
index 0000000000..b925f98197
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/PeopleUpdateQueryTests.cs
@@ -0,0 +1,138 @@
+using System;
+using System.Collections.Generic;
+using System.Data.Common;
+using System.Linq;
+using Emby.Server.Implementations.Data;
+using Jellyfin.Data.Enums;
+using Jellyfin.Database.Implementations.Entities;
+using Jellyfin.Server.Implementations.Item;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Persistence;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Diagnostics;
+using Microsoft.EntityFrameworkCore.Migrations;
+using Microsoft.EntityFrameworkCore.Migrations.Operations;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.Item;
+
+public sealed class PeopleUpdateQueryTests : SqliteDbTestFixture
+{
+    private readonly CommandRecorder _recorder;
+    private readonly Guid _itemId = Guid.NewGuid();
+    private readonly PeopleRepository _people;
+
+    public PeopleUpdateQueryTests()
+        : this(new CommandRecorder())
+    {
+    }
+
+    private PeopleUpdateQueryTests(CommandRecorder recorder)
+        : base(recorder)
+    {
+        _recorder = recorder;
+        using var context = CreateDbContext();
+        context.BaseItems.Add(new BaseItemEntity
+        {
+            Id = _itemId,
+            Name = "Movie",
+            Type = new ItemTypeLookup().BaseItemKindNames[BaseItemKind.Movie]
+        });
+        context.SaveChanges();
+        _people = new PeopleRepository(CreateDbContextFactory(), new ItemTypeLookup(), Mock.Of<IItemQueryHelpers>());
+    }
+
+    [Theory]
+    [InlineData("Hero")]
+    [InlineData("HERO")]
+    public void UnchangedCredits_DoNotWriteOrLookUpAllPeople(string role)
+    {
+        _people.UpdatePeople(_itemId, [new PersonInfo { Name = "Actor", Type = PersonKind.Actor, Role = "Hero" }]);
+        _recorder.Commands.Clear();
+        _people.UpdatePeople(_itemId, [new PersonInfo { Name = "actor", Type = PersonKind.Actor, Role = role }]);
+        Assert.Single(_recorder.Commands);
+        Assert.StartsWith("SELECT", _recorder.Commands[0].Sql, StringComparison.Ordinal);
+        using var context = CreateDbContext();
+        Assert.Equal("Hero", Assert.Single(context.PeopleBaseItemMap).Role);
+    }
+
+    [Fact]
+    public void SortOrderChange_IsPersisted()
+    {
+        _people.UpdatePeople(_itemId, [new PersonInfo { Name = "Actor", Type = PersonKind.Actor, SortOrder = 1 }]);
+        _people.UpdatePeople(_itemId, [new PersonInfo { Name = "Actor", Type = PersonKind.Actor, SortOrder = 2 }]);
+        using var context = CreateDbContext();
+        Assert.Equal(2, Assert.Single(context.PeopleBaseItemMap).SortOrder);
+    }
+
+    [Fact]
+    public void UpdatePeople_GeneratedSqlUsesPeopleNameIndex()
+    {
+        ApplyMigration(new Jellyfin.Server.Implementations.Migrations.AddPeopleNameLowerIndex());
+        _recorder.Commands.Clear();
+        _people.UpdatePeople(_itemId, [
+            new PersonInfo { Name = "Actor A", Type = PersonKind.Actor },
+            new PersonInfo { Name = "Actor B", Type = PersonKind.Actor }
+        ]);
+        var query = Assert.Single(_recorder.Commands, c => c.Sql.Contains("lower(\"p\".\"Name\")", StringComparison.Ordinal));
+        Assert.Contains(Explain(query), line => line.Contains("SEARCH p USING INDEX IX_Peoples_NameLower", StringComparison.Ordinal));
+    }
+
+    private void ApplyMigration(Migration migration)
+    {
+        using var context = CreateDbContext();
+        foreach (var operation in migration.UpOperations.Cast<SqlOperation>())
+        {
+            context.Database.ExecuteSqlRaw(operation.Sql);
+        }
+    }
+
+    private string[] Explain(RecordedCommand query)
+    {
+        using var context = CreateDbContext();
+        using var command = context.Database.GetDbConnection().CreateCommand();
+#pragma warning disable CA2100 // query.Sql is generated by EF Core; query values remain bound parameters.
+        command.CommandText = "EXPLAIN QUERY PLAN " + query.Sql;
+#pragma warning restore CA2100
+        foreach (var value in query.Parameters)
+        {
+            var parameter = command.CreateParameter();
+            parameter.ParameterName = value.Name;
+            parameter.Value = value.Value;
+            command.Parameters.Add(parameter);
+        }
+
+        using var reader = command.ExecuteReader();
+        var plan = new List<string>();
+        while (reader.Read())
+        {
+            plan.Add(reader.GetString(3));
+        }
+
+        return plan.ToArray();
+    }
+
+    private sealed record RecordedCommand(string Sql, (string Name, object? Value)[] Parameters);
+
+    private sealed class CommandRecorder : DbCommandInterceptor
+    {
+        public List<RecordedCommand> Commands { get; } = [];
+
+        public override InterceptionResult<DbDataReader> ReaderExecuting(DbCommand command, CommandEventData eventData, InterceptionResult<DbDataReader> result)
+        {
+            Record(command);
+            return result;
+        }
+
+        public override InterceptionResult<int> NonQueryExecuting(DbCommand command, CommandEventData eventData, InterceptionResult<int> result)
+        {
+            Record(command);
+            return result;
+        }
+
+        private void Record(DbCommand command) => Commands.Add(new RecordedCommand(
+            command.CommandText,
+            command.Parameters.Cast<DbParameter>().Select(p => (p.ParameterName, p.Value)).ToArray()));
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs
index cfc9c9496c..6da176b4f1 100644
--- a/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs
+++ b/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs
@@ -11,6 +11,7 @@ using MediaBrowser.Controller.Configuration;
 using MediaBrowser.Model.Configuration;
 using Microsoft.Data.Sqlite;
 using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Diagnostics;
 using Microsoft.Extensions.Logging.Abstractions;
 using Moq;
 
@@ -26,7 +27,7 @@ public abstract class SqliteDbTestFixture : IDisposable
     private readonly SqliteConnection _connection;
     private readonly DbContextOptions<JellyfinDbContext> _dbOptions;
 
-    protected SqliteDbTestFixture()
+    protected SqliteDbTestFixture(params IInterceptor[] interceptors)
     {
         ApplicationPaths = new Mock<IApplicationPaths>().Object;
 
@@ -35,6 +36,7 @@ public abstract class SqliteDbTestFixture : IDisposable
 
         _dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>()
             .UseSqlite(_connection)
+            .AddInterceptors(interceptors)
             .Options;
 
         using var context = CreateDbContext();