From a67e15bbb36cfbf872699ea730cf0a9d318af39f Mon Sep 17 00:00:00 2001 From: Amelia <77553571+Fesaa@users.noreply.github.com> Date: Fri, 30 May 2025 14:29:03 +0200 Subject: [PATCH] Add more unit tests for the service - fix some found bugs --- API.Tests/Helpers/RandfHelper.cs | 124 +++++++++ .../Services/ReadingProfileServiceTest.cs | 258 ++++++++++++++++++ .../AppUserReadingProfileRepository.cs | 6 + API/Services/ReadingProfileService.cs | 21 +- 4 files changed, 401 insertions(+), 8 deletions(-) create mode 100644 API.Tests/Helpers/RandfHelper.cs diff --git a/API.Tests/Helpers/RandfHelper.cs b/API.Tests/Helpers/RandfHelper.cs new file mode 100644 index 000000000..d8c007df7 --- /dev/null +++ b/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 (); + + /// + /// Returns true if all simple fields are equal + /// + /// + /// + /// fields to ignore, note that the names are very weird sometimes + /// + /// + /// + public static bool AreSimpleFieldsEqual(object obj1, object obj2, IList 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; + } + + /// + /// Sets all simple fields of the given object to a random value + /// + /// + /// Simple is, primitive, string, or enum + /// + 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()); + } +} diff --git a/API.Tests/Services/ReadingProfileServiceTest.cs b/API.Tests/Services/ReadingProfileServiceTest.cs index 5c33af956..bf88aa2fb 100644 --- a/API.Tests/Services/ReadingProfileServiceTest.cs +++ b/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(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(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); + } + + /// + /// 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. + /// + [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(profile); + + Assert.True(RandfHelper.AreSimpleFieldsEqual(dto, newDto, + ["k__BackingField", "k__BackingField", "k__BackingField"])); + } + + + protected override async Task ResetDb() { Context.AppUserReadingProfile.RemoveRange(Context.AppUserReadingProfile); diff --git a/API/Data/Repositories/AppUserReadingProfileRepository.cs b/API/Data/Repositories/AppUserReadingProfileRepository.cs index fd06c4bcd..07aec3c95 100644 --- a/API/Data/Repositories/AppUserReadingProfileRepository.cs +++ b/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); diff --git a/API/Services/ReadingProfileService.cs b/API/Services/ReadingProfileService.cs index e9ae32fe8..72ec9a39c 100644 --- a/API/Services/ReadingProfileService.cs +++ b/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 = new LibraryReadingProfile - { - AppUserId = userId, - LibraryId = libraryId, - }; + libraryProfile.ReadingProfile = profile; + await unitOfWork.CommitAsync(); + return; } - libraryProfile.ReadingProfile = profile; + libraryProfile = new LibraryReadingProfile + { + AppUserId = userId, + LibraryId = libraryId, + 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()) {