Add more unit tests for the service - fix some found bugs

API.Tests/Helpers/RandfHelper.cs

@@ -0,0 +1,124 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+
+namespace API.Tests.Helpers;
+
+public class RandfHelper
+{
+    private static readonly Random Random = new ();
+
+    /// <summary>
+    /// Returns true if all simple fields are equal
+    /// </summary>
+    /// <param name="obj1"></param>
+    /// <param name="obj2"></param>
+    /// <param name="ignoreFields">fields to ignore, note that the names are very weird sometimes</param>
+    /// <returns></returns>
+    /// <exception cref="ArgumentNullException"></exception>
+    /// <exception cref="ArgumentException"></exception>
+    public static bool AreSimpleFieldsEqual(object obj1, object obj2, IList<string> ignoreFields)
+    {
+        if (obj1 == null || obj2 == null)
+            throw new ArgumentNullException("Neither object can be null.");
+
+        Type type1 = obj1.GetType();
+        Type type2 = obj2.GetType();
+
+        if (type1 != type2)
+            throw new ArgumentException("Objects must be of the same type.");
+
+        FieldInfo[] fields = type1.GetFields(BindingFlags.Public | BindingFlags.Instance | BindingFlags.NonPublic);
+
+        foreach (var field in fields)
+        {
+            if (field.IsInitOnly) continue;
+            if (ignoreFields.Contains(field.Name)) continue;
+
+            Type fieldType = field.FieldType;
+
+            if (IsRelevantType(fieldType))
+            {
+                object value1 = field.GetValue(obj1);
+                object value2 = field.GetValue(obj2);
+
+                if (!Equals(value1, value2))
+                {
+                    throw new ArgumentException("Fields must be of the same type: " +  field.Name + " was  " + value1 + " and  " + value2);
+                }
+            }
+        }
+
+        return true;
+    }
+
+    private static bool IsRelevantType(Type type)
+    {
+        return type.IsPrimitive
+               || type == typeof(string)
+               || type.IsEnum;
+    }
+
+    /// <summary>
+    /// Sets all simple fields of the given object to a random value
+    /// </summary>
+    /// <param name="obj"></param>
+    /// <remarks>Simple is, primitive, string, or enum</remarks>
+    /// <exception cref="ArgumentNullException"></exception>
+    public static void SetRandomValues(object obj)
+    {
+        if (obj == null) throw new ArgumentNullException(nameof(obj));
+
+        Type type = obj.GetType();
+        FieldInfo[] fields = type.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
+
+        foreach (var field in fields)
+        {
+            if (field.IsInitOnly) continue; // Skip readonly fields
+
+            object value = GenerateRandomValue(field.FieldType);
+            if (value != null)
+            {
+                field.SetValue(obj, value);
+            }
+        }
+    }
+
+    private static object GenerateRandomValue(Type type)
+    {
+        if (type == typeof(int))
+            return Random.Next();
+        if (type == typeof(float))
+            return (float)Random.NextDouble() * 100;
+        if (type == typeof(double))
+            return Random.NextDouble() * 100;
+        if (type == typeof(bool))
+            return Random.Next(2) == 1;
+        if (type == typeof(char))
+            return (char)Random.Next('A', 'Z' + 1);
+        if (type == typeof(byte))
+            return (byte)Random.Next(0, 256);
+        if (type == typeof(short))
+            return (short)Random.Next(short.MinValue, short.MaxValue);
+        if (type == typeof(long))
+            return (long)(Random.NextDouble() * long.MaxValue);
+        if (type == typeof(string))
+            return GenerateRandomString(10);
+        if (type.IsEnum)
+        {
+            var values = Enum.GetValues(type);
+            return values.GetValue(Random.Next(values.Length));
+        }
+
+        // Unsupported type
+        return null;
+    }
+
+    private static string GenerateRandomString(int length)
+    {
+        const string chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
+        return new string(Enumerable.Repeat(chars, length)
+            .Select(s => s[Random.Next(s.Length)]).ToArray());
+    }
+}

API.Tests/Services/ReadingProfileServiceTest.cs

@@ -4,8 +4,10 @@ using API.Data.Repositories;
 using API.DTOs;
 using API.Entities;
 using API.Entities.Enums;
+using API.Extensions.QueryExtensions;
 using API.Helpers.Builders;
 using API.Services;
+using API.Tests.Helpers;
 using Kavita.Common;
 using Microsoft.EntityFrameworkCore;
 using NSubstitute;
@@ -131,6 +133,43 @@ public class ReadingProfileServiceTest: AbstractDbTest
         Assert.True(profile.Implicit);
     }
 
+    [Fact]
+    public async Task UpdateImplicitReadingProfile_DoesnotCreateNew()
+    {
+        await ResetDb();
+        var (rps, user, _, series) = await Setup();
+
+        var dto = new UserReadingProfileDto
+        {
+            ReaderMode = ReaderMode.Webtoon,
+            ScalingOption = ScalingOption.FitToHeight,
+            WidthOverride = 53,
+        };
+
+        await rps.UpdateImplicitReadingProfile(user.Id, series.Id, dto);
+
+        var profile = await UnitOfWork.AppUserReadingProfileRepository.GetProfileForSeries(user.Id, series.Id);
+        Assert.NotNull(profile);
+        Assert.Contains(profile.Series, s => s.SeriesId == series.Id);
+        Assert.True(profile.Implicit);
+
+        dto = new UserReadingProfileDto
+        {
+            ReaderMode = ReaderMode.LeftRight,
+        };
+
+        await rps.UpdateImplicitReadingProfile(user.Id, series.Id, dto);
+        profile = await UnitOfWork.AppUserReadingProfileRepository.GetProfileForSeries(user.Id, series.Id);
+        Assert.NotNull(profile);
+        Assert.Contains(profile.Series, s => s.SeriesId == series.Id);
+        Assert.True(profile.Implicit);
+        Assert.Equal(ReaderMode.LeftRight, profile.ReaderMode);
+
+        var implicitCount = await Context.AppUserReadingProfile
+            .Where(p => p.Implicit).CountAsync();
+        Assert.Equal(1, implicitCount);
+    }
+
     [Fact]
     public async Task GetCorrectProfile()
     {
@@ -356,6 +395,225 @@ public class ReadingProfileServiceTest: AbstractDbTest
         Assert.Equal(0, implicitCount);
     }
 
+    [Fact]
+    public async Task AddDeletesImplicit()
+    {
+        await ResetDb();
+        var (rps, user, lib, series) = await Setup();
+
+        var implicitProfile = Mapper.Map<UserReadingProfileDto>(new AppUserReadingProfileBuilder(user.Id)
+            .Build());
+
+        var profile = new AppUserReadingProfileBuilder(user.Id)
+            .WithName("Profile 1")
+            .Build();
+        Context.AppUserReadingProfile.Add(profile);
+        await UnitOfWork.CommitAsync();
+
+        await rps.UpdateImplicitReadingProfile(user.Id, series.Id, implicitProfile);
+
+        var seriesProfile = await rps.GetReadingProfileForSeries(user.Id, series.Id);
+        Assert.NotNull(seriesProfile);
+        Assert.True(seriesProfile.Implicit);
+
+        await rps.AddProfileToSeries(user.Id, profile.Id, series.Id);
+
+        seriesProfile = await rps.GetReadingProfileForSeries(user.Id, series.Id);
+        Assert.NotNull(seriesProfile);
+        Assert.False(seriesProfile.Implicit);
+
+        var implicitCount = await Context.AppUserReadingProfile
+            .Where(p => p.Implicit).CountAsync();
+        Assert.Equal(0, implicitCount);
+    }
+
+    [Fact]
+    public async Task SetDefault()
+    {
+        await ResetDb();
+        var (rps, user, lib, series) = await Setup();
+
+        var profile = new AppUserReadingProfileBuilder(user.Id)
+            .WithName("Profile 1")
+            .Build();
+
+        Context.AppUserReadingProfile.Add(profile);
+        await UnitOfWork.CommitAsync();
+
+        await rps.SetDefaultReadingProfile(user.Id, profile.Id);
+
+        var newSeries = new SeriesBuilder("New Series").Build();
+        lib.Series.Add(newSeries);
+        await UnitOfWork.CommitAsync();
+
+        var seriesProfile = await rps.GetReadingProfileForSeries(user.Id, series.Id);
+        Assert.NotNull(seriesProfile);
+        Assert.Equal(profile.Id, seriesProfile.Id);
+    }
+
+    [Fact]
+    public async Task CreateReadingProfile()
+    {
+        await ResetDb();
+        var (rps, user, lib, series) = await Setup();
+
+        var dto = new UserReadingProfileDto
+        {
+            Name = "Profile 1",
+            ReaderMode = ReaderMode.LeftRight,
+            EmulateBook = false,
+        };
+
+        await rps.CreateReadingProfile(user.Id, dto);
+
+        var dto2 = new UserReadingProfileDto
+        {
+            Name = "Profile 2",
+            ReaderMode = ReaderMode.LeftRight,
+            EmulateBook = false,
+        };
+
+        await rps.CreateReadingProfile(user.Id, dto2);
+
+        var dto3 = new UserReadingProfileDto
+        {
+            Name = "Profile 1", // Not unique name
+            ReaderMode = ReaderMode.LeftRight,
+            EmulateBook = false,
+        };
+
+        await Assert.ThrowsAsync<KavitaException>(async () =>
+        {
+            await rps.CreateReadingProfile(user.Id, dto3);
+        });
+
+        var allProfiles = Context.AppUserReadingProfile.ToList();
+        Assert.Equal(2, allProfiles.Count);
+    }
+
+    [Fact]
+    public async Task ClearSeriesProfile_RemovesImplicitAndUnlinksExplicit()
+    {
+        await ResetDb();
+        var (rps, user, _, series) = await Setup();
+
+        var implicitProfile = new AppUserReadingProfileBuilder(user.Id)
+            .WithSeries(series)
+            .WithImplicit(true)
+            .WithName("Implicit Profile")
+            .Build();
+
+        var explicitProfile = new AppUserReadingProfileBuilder(user.Id)
+            .WithSeries(series)
+            .WithImplicit(false)
+            .WithName("Explicit Profile")
+            .Build();
+
+        Context.AppUserReadingProfile.Add(implicitProfile);
+        Context.AppUserReadingProfile.Add(explicitProfile);
+        await UnitOfWork.CommitAsync();
+
+        var allBefore = await UnitOfWork.AppUserReadingProfileRepository.GetAllProfilesForSeries(user.Id, series.Id, ReadingProfileIncludes.Series);
+        Assert.Equal(2, allBefore.Count);
+
+        await rps.ClearSeriesProfile(user.Id, series.Id);
+
+        var remainingProfiles = await Context.AppUserReadingProfile.Includes(ReadingProfileIncludes.Series).ToListAsync();
+        Assert.Single(remainingProfiles);
+        Assert.Equal("Explicit Profile", remainingProfiles[0].Name);
+        Assert.Empty(remainingProfiles[0].Series);
+
+        var profilesForSeries = await UnitOfWork.AppUserReadingProfileRepository.GetAllProfilesForSeries(user.Id, series.Id, ReadingProfileIncludes.Series);
+        Assert.Empty(profilesForSeries);
+    }
+
+    [Fact]
+    public async Task AddProfileToLibrary_AddsAndOverridesExisting()
+    {
+        await ResetDb();
+        var (rps, user, lib, _) = await Setup();
+
+        var profile = new AppUserReadingProfileBuilder(user.Id)
+            .WithName("Library Profile")
+            .Build();
+        Context.AppUserReadingProfile.Add(profile);
+        await UnitOfWork.CommitAsync();
+
+        await rps.AddProfileToLibrary(user.Id, profile.Id, lib.Id);
+        await UnitOfWork.CommitAsync();
+
+        var linkedProfile = await UnitOfWork.AppUserReadingProfileRepository.GetProfileForLibrary(user.Id, lib.Id, ReadingProfileIncludes.Library);
+        Assert.NotNull(linkedProfile);
+        Assert.Equal(profile.Id, linkedProfile.Id);
+
+        var newProfile = new AppUserReadingProfileBuilder(user.Id)
+            .WithName("New Profile")
+            .Build();
+        Context.AppUserReadingProfile.Add(newProfile);
+        await UnitOfWork.CommitAsync();
+
+        await rps.AddProfileToLibrary(user.Id, newProfile.Id, lib.Id);
+        await UnitOfWork.CommitAsync();
+
+        linkedProfile = await UnitOfWork.AppUserReadingProfileRepository.GetProfileForLibrary(user.Id, lib.Id, ReadingProfileIncludes.Library);
+        Assert.NotNull(linkedProfile);
+        Assert.Equal(newProfile.Id, linkedProfile.Id);
+    }
+
+    [Fact]
+    public async Task ClearLibraryProfile_RemovesImplicitOrUnlinksExplicit()
+    {
+        await ResetDb();
+        var (rps, user, lib, _) = await Setup();
+
+        var implicitProfile = new AppUserReadingProfileBuilder(user.Id)
+            .WithImplicit(true)
+            .WithLibrary(lib)
+            .Build();
+        Context.AppUserReadingProfile.Add(implicitProfile);
+        await UnitOfWork.CommitAsync();
+
+        await rps.ClearLibraryProfile(user.Id, lib.Id);
+        var profile = await UnitOfWork.AppUserReadingProfileRepository.GetProfileForLibrary(user.Id, lib.Id, ReadingProfileIncludes.Library);
+        Assert.Null(profile);
+
+        var explicitProfile = new AppUserReadingProfileBuilder(user.Id)
+            .WithLibrary(lib)
+            .Build();
+        Context.AppUserReadingProfile.Add(explicitProfile);
+        await UnitOfWork.CommitAsync();
+
+        await rps.ClearLibraryProfile(user.Id, lib.Id);
+        profile = await UnitOfWork.AppUserReadingProfileRepository.GetProfileForLibrary(user.Id, lib.Id, ReadingProfileIncludes.Library);
+        Assert.Null(profile);
+
+        var stillExists = await Context.AppUserReadingProfile.FindAsync(explicitProfile.Id);
+        Assert.NotNull(stillExists);
+    }
+
+    /// <summary>
+    /// As response to #3793, I'm not sure if we want to keep this. It's not the most nice. But I think the idea of this test
+    /// is worth having.
+    /// </summary>
+    [Fact]
+    public void UpdateFields_UpdatesAll()
+    {
+        var profile = new AppUserReadingProfile();
+        var dto = new UserReadingProfileDto();
+
+        RandfHelper.SetRandomValues(profile);
+        RandfHelper.SetRandomValues(dto);
+
+        ReadingProfileService.UpdateReaderProfileFields(profile, dto);
+
+        var newDto = Mapper.Map<UserReadingProfileDto>(profile);
+
+        Assert.True(RandfHelper.AreSimpleFieldsEqual(dto, newDto,
+            ["<Id>k__BackingField", "<UserId>k__BackingField", "<Implicit>k__BackingField"]));
+    }
+
+
+
     protected override async Task ResetDb()
     {
         Context.AppUserReadingProfile.RemoveRange(Context.AppUserReadingProfile);

API/Data/Repositories/AppUserReadingProfileRepository.cs

@@ -47,6 +47,7 @@ public interface IAppUserReadingProfileRepository
 
     void Add(AppUserReadingProfile readingProfile);
     void Add(SeriesReadingProfile readingProfile);
+    void Add(LibraryReadingProfile readingProfile);
     void Attach(AppUserReadingProfile readingProfile);
     void Update(AppUserReadingProfile readingProfile);
     void Update(SeriesReadingProfile readingProfile);
@@ -184,6 +185,11 @@ public class AppUserReadingProfileRepository(DataContext context, IMapper mapper
         context.SeriesReadingProfile.Add(readingProfile);
     }
 
+    public void Add(LibraryReadingProfile readingProfile)
+    {
+        context.LibraryReadingProfile.Add(readingProfile);
+    }
+
     public void Attach(AppUserReadingProfile readingProfile)
     {
         context.AppUserReadingProfile.Attach(readingProfile);

API/Services/ReadingProfileService.cs

@@ -286,16 +286,21 @@ public class ReadingProfileService(IUnitOfWork unitOfWork, ILocalizationService
         if (profile.UserId != userId) throw new UnauthorizedAccessException();
 
         var libraryProfile = await unitOfWork.AppUserReadingProfileRepository.GetLibraryProfile(userId, libraryId);
-        if (libraryProfile == null)
+        if (libraryProfile != null)
         {
+            libraryProfile.ReadingProfile = profile;
+            await unitOfWork.CommitAsync();
+            return;
+        }
+
         libraryProfile = new LibraryReadingProfile
         {
             AppUserId = userId,
             LibraryId = libraryId,
+            ReadingProfile = profile,
         };
-        }
 
-        libraryProfile.ReadingProfile = profile;
+        unitOfWork.AppUserReadingProfileRepository.Add(libraryProfile);
         await unitOfWork.CommitAsync();
     }
 
@@ -317,7 +322,7 @@ public class ReadingProfileService(IUnitOfWork unitOfWork, ILocalizationService
         await unitOfWork.CommitAsync();
     }
 
-    private static void UpdateReaderProfileFields(AppUserReadingProfile existingProfile, UserReadingProfileDto dto, bool updateName = true)
+    public static void UpdateReaderProfileFields(AppUserReadingProfile existingProfile, UserReadingProfileDto dto, bool updateName = true)
     {
         if (updateName && !string.IsNullOrEmpty(dto.Name) && existingProfile.NormalizedName != dto.Name.ToNormalized())
         {