diff options
| author | TowyTowy <towy@airreps.link> | 2026-07-09 13:39:09 +0200 |
|---|---|---|
| committer | TowyTowy <towy@airreps.link> | 2026-07-09 13:39:09 +0200 |
| commit | 2326ecdedc042f7f1c2ce2a5da409377c09778d7 (patch) | |
| tree | e6dee88eb93b9e46ec4912f998cd25f8a59bca23 | |
| parent | 53aafcd38e1f4558ff18f5258d0d46b3a0565783 (diff) | |
Fix profile image being impossible to clear when its in-memory key is temporary
ClearProfileImageAsync removed the ProfileImage instance attached to the
passed-in User, but that instance can carry a stale, never-persisted
(temporary) key because UpdateUserAsync creates the persisted image on a
separately loaded entity and never copies the generated key back.
Removing that detached entity on a fresh DbContext made EF Core throw
InvalidOperationException ('ImageInfo.Id has a temporary value'), leaving
the profile image impossible to delete or replace.
Load the tracked, persisted user and remove its actual ProfileImage,
matching the removal pattern already used in UpdateUserAsync. Adds
regression tests covering the temporary-key case and the no-image no-op
(the first fails before this change and passes after).
Fixes #13137
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| -rw-r--r-- | Jellyfin.Server.Implementations/Users/UserManager.cs | 16 | ||||
| -rw-r--r-- | tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerProfileImageTests.cs | 142 |
2 files changed, 156 insertions, 2 deletions
diff --git a/Jellyfin.Server.Implementations/Users/UserManager.cs b/Jellyfin.Server.Implementations/Users/UserManager.cs index 80722af106..b42fae751a 100644 --- a/Jellyfin.Server.Implementations/Users/UserManager.cs +++ b/Jellyfin.Server.Implementations/Users/UserManager.cs @@ -882,8 +882,20 @@ namespace Jellyfin.Server.Implementations.Users var dbContext = await _dbProvider.CreateDbContextAsync().ConfigureAwait(false); await using (dbContext.ConfigureAwait(false)) { - dbContext.Remove(user.ProfileImage); - await dbContext.SaveChangesAsync().ConfigureAwait(false); + // Remove the tracked profile image loaded from the database instead of the + // detached instance on the passed in user. That instance can carry a stale, + // never-persisted (temporary) key, which makes EF Core throw when it is marked + // for deletion, leaving the profile image impossible to clear or replace. + var dbUser = await UserQuery(dbContext) + .AsTracking() + .FirstOrDefaultAsync(u => u.Id == user.Id) + .ConfigureAwait(false); + if (dbUser?.ProfileImage is not null) + { + dbContext.Remove(dbUser.ProfileImage); + dbUser.ProfileImage = null; + await dbContext.SaveChangesAsync().ConfigureAwait(false); + } } user.ProfileImage = null; diff --git a/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerProfileImageTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerProfileImageTests.cs new file mode 100644 index 0000000000..cb714a4014 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Users/UserManagerProfileImageTests.cs @@ -0,0 +1,142 @@ +using System; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Database.Implementations.Locking; +using Jellyfin.Database.Providers.Sqlite; +using Jellyfin.Server.Implementations.Users; +using MediaBrowser.Common; +using MediaBrowser.Common.Net; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Authentication; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Drawing; +using MediaBrowser.Controller.Events; +using MediaBrowser.Controller.Library; +using MediaBrowser.Model.Cryptography; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Users +{ + public sealed class UserManagerProfileImageTests : IDisposable + { + private readonly SqliteConnection _connection; + private readonly DbContextOptions<JellyfinDbContext> _dbOptions; + private readonly UserManager _userManager; + + public UserManagerProfileImageTests() + { + _connection = new SqliteConnection("Data Source=:memory:"); + _connection.Open(); + + _dbOptions = new DbContextOptionsBuilder<JellyfinDbContext>() + .UseSqlite(_connection) + .Options; + + // Create the schema + using var ctx = CreateDbContext(); + ctx.Database.EnsureCreated(); + + var factory = new Mock<IDbContextFactory<JellyfinDbContext>>(); + factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + factory.Setup(f => f.CreateDbContextAsync(It.IsAny<CancellationToken>())) + .ReturnsAsync(CreateDbContext); + + var cryptoProvider = new Mock<ICryptoProvider>(); + var configManager = new Mock<IServerConfigurationManager>(); + var appPaths = new Mock<IServerApplicationPaths>(); + appPaths.Setup(x => x.ProgramDataPath).Returns(Path.GetTempPath()); + configManager.Setup(x => x.ApplicationPaths).Returns(appPaths.Object); + + var appHost = new Mock<IApplicationHost>(); + + var defaultAuthProvider = new DefaultAuthenticationProvider( + NullLogger<DefaultAuthenticationProvider>.Instance, + cryptoProvider.Object); + var invalidAuthProvider = new InvalidAuthProvider(); + var defaultPasswordResetProvider = new DefaultPasswordResetProvider( + configManager.Object, + appHost.Object); + + _userManager = new UserManager( + factory.Object, + new NoopEventManager(), + new Mock<INetworkManager>().Object, + appHost.Object, + new Mock<IImageProcessor>().Object, + NullLogger<UserManager>.Instance, + configManager.Object, + new IPasswordResetProvider[] { defaultPasswordResetProvider }, + new IAuthenticationProvider[] { defaultAuthProvider, invalidAuthProvider }); + } + + public void Dispose() + { + _userManager.Dispose(); + _connection.Dispose(); + } + + private JellyfinDbContext CreateDbContext() + { + return new JellyfinDbContext( + _dbOptions, + NullLogger<JellyfinDbContext>.Instance, + new SqliteDatabaseProvider(null!, NullLogger<SqliteDatabaseProvider>.Instance), + new NoLockBehavior(NullLogger<NoLockBehavior>.Instance)); + } + + [Fact] + public async Task ClearProfileImageAsync_WhenInMemoryImageHasTemporaryKey_RemovesPersistedImage() + { + var user = await _userManager.CreateUserAsync("profileimageuser"); + + // Assign a profile image the same way the image endpoint does and persist it. + // UpdateUserAsync creates the persisted ImageInfo on a separately loaded db entity, + // so the in-memory instance below is never assigned the database generated key. + user.ProfileImage = new ImageInfo(Path.Combine(Path.GetTempPath(), "profile.png")); + await _userManager.UpdateUserAsync(user); + + // Precondition reproducing the bug: the in-memory image still carries the default, + // never-persisted (temporary) key, while a real image row exists in the database. + Assert.Equal(0, user.ProfileImage.Id); + Assert.NotNull(_userManager.GetUserById(user.Id)!.ProfileImage); + + // This used to throw InvalidOperationException: + // "The property 'ImageInfo.Id' has a temporary value while attempting to change the entity's state to 'Deleted'." + var exception = await Record.ExceptionAsync(() => _userManager.ClearProfileImageAsync(user)); + + Assert.Null(exception); + Assert.Null(user.ProfileImage); + Assert.Null(_userManager.GetUserById(user.Id)!.ProfileImage); + } + + [Fact] + public async Task ClearProfileImageAsync_WhenNoProfileImage_DoesNothing() + { + var user = await _userManager.CreateUserAsync("noprofileimageuser"); + + var exception = await Record.ExceptionAsync(() => _userManager.ClearProfileImageAsync(user)); + + Assert.Null(exception); + Assert.Null(user.ProfileImage); + } + + private sealed class NoopEventManager : IEventManager + { + public void Publish<T>(T eventArgs) + where T : EventArgs + { + } + + public Task PublishAsync<T>(T eventArgs) + where T : EventArgs + => Task.CompletedTask; + } + } +} |
