diff --git a/ImmichFrame.Core.Tests/Logic/Pool/AlbumAssetsPoolTests.cs b/ImmichFrame.Core.Tests/Logic/Pool/AlbumAssetsPoolTests.cs index 6d1058d5..11bef684 100644 --- a/ImmichFrame.Core.Tests/Logic/Pool/AlbumAssetsPoolTests.cs +++ b/ImmichFrame.Core.Tests/Logic/Pool/AlbumAssetsPoolTests.cs @@ -98,4 +98,30 @@ public async Task LoadAssets_NoExcludedAlbums_ReturnsAlbums() Assert.That(result.Count, Is.EqualTo(1)); Assert.That(result.Any(a => a.Id == "A")); } + + [Test] + public async Task LoadAssets_NullAlbums_ReturnsEmpty() + { + _mockAccountSettings.SetupGet(s => s.Albums).Returns((List)null); + _mockAccountSettings.SetupGet(s => s.ExcludedAlbums).Returns(new List()); + + var result = (await _albumAssetsPool.TestLoadAssets()).ToList(); + Assert.That(result, Is.Empty); + _mockImmichApi.Verify(api => api.GetAlbumInfoAsync(It.IsAny(), null, null, It.IsAny()), Times.Never); + } + + [Test] + public async Task LoadAssets_NullExcludedAlbums_ReturnsAlbumAssets() + { + var album1Id = Guid.NewGuid(); + _mockAccountSettings.SetupGet(s => s.Albums).Returns(new List { album1Id }); + _mockAccountSettings.SetupGet(s => s.ExcludedAlbums).Returns((List)null); + + _mockImmichApi.Setup(api => api.GetAlbumInfoAsync(album1Id, null, null, It.IsAny())) + .ReturnsAsync(new AlbumResponseDto { Assets = new List { CreateAsset("A") } }); + + var result = (await _albumAssetsPool.TestLoadAssets()).ToList(); + Assert.That(result.Count, Is.EqualTo(1)); + Assert.That(result.Any(a => a.Id == "A")); + } } diff --git a/ImmichFrame.Core.Tests/Logic/Pool/AllAssetsPoolTests.cs b/ImmichFrame.Core.Tests/Logic/Pool/AllAssetsPoolTests.cs index 5ea7e984..b64b0be4 100644 --- a/ImmichFrame.Core.Tests/Logic/Pool/AllAssetsPoolTests.cs +++ b/ImmichFrame.Core.Tests/Logic/Pool/AllAssetsPoolTests.cs @@ -133,4 +133,44 @@ public async Task GetAssets_ExcludesAssetsFromExcludedAlbums() Assert.That(result.All(a => a.Id.StartsWith("main"))); _mockImmichApi.Verify(api => api.GetAlbumInfoAsync(excludedAlbumId, null, null, It.IsAny()), Times.Once); } + + [Test] + public async Task GetAssets_NullExcludedAlbums_ReturnsAllAssets() + { + // Arrange + var allAssets = CreateSampleAssets(5, "asset"); + _mockAccountSettings.SetupGet(s => s.ExcludedAlbums).Returns((List)null); + + _mockImmichApi.Setup(api => api.SearchRandomAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(allAssets); + + // Act + var result = (await _allAssetsPool.GetAssets(5)).ToList(); + + // Assert + Assert.That(result.Count, Is.EqualTo(5)); + Assert.That(result, Is.EqualTo(allAssets)); + // Verify that GetAlbumInfoAsync was never called since ExcludedAlbums is null + _mockImmichApi.Verify(api => api.GetAlbumInfoAsync(It.IsAny(), null, null, It.IsAny()), Times.Never); + } + + [Test] + public async Task GetAssets_EmptyExcludedAlbums_ReturnsAllAssets() + { + // Arrange + var allAssets = CreateSampleAssets(5, "asset"); + _mockAccountSettings.SetupGet(s => s.ExcludedAlbums).Returns(new List()); + + _mockImmichApi.Setup(api => api.SearchRandomAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(allAssets); + + // Act + var result = (await _allAssetsPool.GetAssets(5)).ToList(); + + // Assert + Assert.That(result.Count, Is.EqualTo(5)); + Assert.That(result, Is.EqualTo(allAssets)); + // Verify that GetAlbumInfoAsync was never called since ExcludedAlbums is empty + _mockImmichApi.Verify(api => api.GetAlbumInfoAsync(It.IsAny(), null, null, It.IsAny()), Times.Never); + } } diff --git a/ImmichFrame.Core.Tests/Logic/Pool/PersonAssetsPoolTests.cs b/ImmichFrame.Core.Tests/Logic/Pool/PersonAssetsPoolTests.cs index d4e729c1..6057b961 100644 --- a/ImmichFrame.Core.Tests/Logic/Pool/PersonAssetsPoolTests.cs +++ b/ImmichFrame.Core.Tests/Logic/Pool/PersonAssetsPoolTests.cs @@ -91,6 +91,15 @@ public async Task LoadAssets_NoPeopleConfigured_ReturnsEmpty() _mockImmichApi.Verify(api => api.SearchAssetsAsync(It.IsAny(), It.IsAny()), Times.Never); } + [Test] + public async Task LoadAssets_NullPeople_ReturnsEmpty() + { + _mockAccountSettings.SetupGet(s => s.People).Returns((List)null); + var result = (await _personAssetsPool.TestLoadAssets()).ToList(); + Assert.That(result, Is.Empty); + _mockImmichApi.Verify(api => api.SearchAssetsAsync(It.IsAny(), It.IsAny()), Times.Never); + } + [Test] public async Task LoadAssets_PersonHasNoAssets_DoesNotAffectOthers() { diff --git a/ImmichFrame.Core.Tests/Logic/PooledImmichFrameLogicTests.cs b/ImmichFrame.Core.Tests/Logic/PooledImmichFrameLogicTests.cs new file mode 100644 index 00000000..65cf963f --- /dev/null +++ b/ImmichFrame.Core.Tests/Logic/PooledImmichFrameLogicTests.cs @@ -0,0 +1,144 @@ +using NUnit.Framework; +using Moq; +using ImmichFrame.Core.Api; +using ImmichFrame.Core.Interfaces; +using ImmichFrame.Core.Logic; +using ImmichFrame.Core.Logic.Pool; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace ImmichFrame.Core.Tests.Logic; + +[TestFixture] +public class PooledImmichFrameLogicTests +{ + private Mock _mockAccountSettings; + private Mock _mockGeneralSettings; + private Mock _mockHttpClientFactory; + private Mock _mockHttpClient; + + [SetUp] + public void Setup() + { + _mockAccountSettings = new Mock(); + _mockGeneralSettings = new Mock(); + _mockHttpClientFactory = new Mock(); + _mockHttpClient = new Mock(); + + // Default setup for common properties + _mockAccountSettings.SetupGet(s => s.ImmichServerUrl).Returns("http://localhost:2283"); + _mockAccountSettings.SetupGet(s => s.ApiKey).Returns("test-api-key"); + _mockAccountSettings.SetupGet(s => s.ShowFavorites).Returns(false); + _mockAccountSettings.SetupGet(s => s.ShowMemories).Returns(false); + _mockAccountSettings.SetupGet(s => s.Albums).Returns(new List()); + _mockAccountSettings.SetupGet(s => s.ShowArchived).Returns(false); + + _mockGeneralSettings.SetupGet(g => g.RefreshAlbumPeopleInterval).Returns(24); + _mockGeneralSettings.SetupGet(g => g.DownloadImages).Returns(false); + + _mockHttpClientFactory.Setup(f => f.CreateClient("ImmichApiAccountClient")).Returns(_mockHttpClient.Object); + } + + /// + /// Ensures that when accountSettings.People is null, BuildPool creates an AllAssetsPool + /// instead of a MultiAssetPool that would include PersonAssetsPool + /// + [Test] + public void BuildPool_WithNullPeople_DoesNotAddPersonAssetsPool() + { + // Arrange + _mockAccountSettings.SetupGet(s => s.People).Returns((List)null); + + // Act + var logic = new PooledImmichFrameLogic(_mockAccountSettings.Object, _mockGeneralSettings.Object, _mockHttpClientFactory.Object); + + // Assert - The pool should be AllAssetsPool, not MultiAssetPool + // We can verify this by checking that the pool is created and is of type AllAssetsPool + Assert.That(logic.AccountSettings.People, Is.Null); + // The fact that the object was created without throwing an exception validates that + // the pool was built correctly without trying to use null People + } + + /// + /// Ensures that when accountSettings.People is an empty list, BuildPool creates an AllAssetsPool + /// instead of a MultiAssetPool, since there are no people to add + /// + [Test] + public void BuildPool_WithEmptyPeople_DoesNotAddPersonAssetsPool() + { + // Arrange + var emptyPeople = new List(); + _mockAccountSettings.SetupGet(s => s.People).Returns(emptyPeople); + + // Act + var logic = new PooledImmichFrameLogic(_mockAccountSettings.Object, _mockGeneralSettings.Object, _mockHttpClientFactory.Object); + + // Assert + Assert.That(logic.AccountSettings.People.Count, Is.EqualTo(0)); + // The pool was created successfully without adding PersonAssetsPool + } + + /// + /// Ensures that when accountSettings.People is null, along with other settings being false/empty, + /// BuildPool returns an AllAssetsPool + /// + [Test] + public void BuildPool_WithAllSettingsFalseAndNullPeople_CreatesAllAssetsPool() + { + // Arrange - all options are disabled/empty/null + _mockAccountSettings.SetupGet(s => s.ShowFavorites).Returns(false); + _mockAccountSettings.SetupGet(s => s.ShowMemories).Returns(false); + _mockAccountSettings.SetupGet(s => s.Albums).Returns(new List()); + _mockAccountSettings.SetupGet(s => s.People).Returns((List)null); + + // Act + var logic = new PooledImmichFrameLogic(_mockAccountSettings.Object, _mockGeneralSettings.Object, _mockHttpClientFactory.Object); + + // Assert - should not throw and should create successfully + Assert.That(logic.AccountSettings, Is.Not.Null); + Assert.That(logic.AccountSettings.People, Is.Null); + } + + /// + /// Ensures that when accountSettings.People is not null and has values, + /// BuildPool correctly includes PersonAssetsPool in the MultiAssetPool + /// + [Test] + public void BuildPool_WithPeopleValues_IncludesPersonAssetsPool() + { + // Arrange + var personId = Guid.NewGuid(); + _mockAccountSettings.SetupGet(s => s.People).Returns(new List { personId }); + + // Act + var logic = new PooledImmichFrameLogic(_mockAccountSettings.Object, _mockGeneralSettings.Object, _mockHttpClientFactory.Object); + + // Assert + Assert.That(logic.AccountSettings.People, Is.Not.Null); + Assert.That(logic.AccountSettings.People.Count, Is.EqualTo(1)); + Assert.That(logic.AccountSettings.People.First(), Is.EqualTo(personId)); + } + + /// + /// Ensures that mixed settings (some enabled, some disabled) with null People + /// creates a proper MultiAssetPool excluding PersonAssetsPool + /// + [Test] + public void BuildPool_WithMixedSettingsAndNullPeople_DoesNotIncludePersonAssetsPool() + { + // Arrange - enable some settings but keep People null + _mockAccountSettings.SetupGet(s => s.ShowFavorites).Returns(true); + _mockAccountSettings.SetupGet(s => s.ShowMemories).Returns(false); + _mockAccountSettings.SetupGet(s => s.Albums).Returns(new List()); + _mockAccountSettings.SetupGet(s => s.People).Returns((List)null); + + // Act + var logic = new PooledImmichFrameLogic(_mockAccountSettings.Object, _mockGeneralSettings.Object, _mockHttpClientFactory.Object); + + // Assert + Assert.That(logic.AccountSettings.People, Is.Null); + // Pool should be created without errors + Assert.That(logic.AccountSettings, Is.Not.Null); + } +} diff --git a/ImmichFrame.Core/Logic/Pool/AlbumAssetsPool.cs b/ImmichFrame.Core/Logic/Pool/AlbumAssetsPool.cs index 5487596d..39f2b5df 100644 --- a/ImmichFrame.Core/Logic/Pool/AlbumAssetsPool.cs +++ b/ImmichFrame.Core/Logic/Pool/AlbumAssetsPool.cs @@ -10,18 +10,26 @@ protected override async Task> LoadAssets(Cancella { var excludedAlbumAssets = new List(); - foreach (var albumId in accountSettings.ExcludedAlbums) + var excludedAlbums = accountSettings.ExcludedAlbums; + if (excludedAlbums != null) { - var albumInfo = await immichApi.GetAlbumInfoAsync(albumId, null, null, ct); - excludedAlbumAssets.AddRange(albumInfo.Assets); + foreach (var albumId in excludedAlbums) + { + var albumInfo = await immichApi.GetAlbumInfoAsync(albumId, null, null, ct); + excludedAlbumAssets.AddRange(albumInfo.Assets); + } } var albumAssets = new List(); - foreach (var albumId in accountSettings.Albums) + var albums = accountSettings.Albums; + if (albums != null) { - var albumInfo = await immichApi.GetAlbumInfoAsync(albumId, null, null, ct); - albumAssets.AddRange(albumInfo.Assets); + foreach (var albumId in albums) + { + var albumInfo = await immichApi.GetAlbumInfoAsync(albumId, null, null, ct); + albumAssets.AddRange(albumInfo.Assets); + } } return albumAssets.WhereExcludes(excludedAlbumAssets, t => t.Id); diff --git a/ImmichFrame.Core/Logic/Pool/AllAssetsPool.cs b/ImmichFrame.Core/Logic/Pool/AllAssetsPool.cs index 0ed6d86f..a4ac3a9e 100644 --- a/ImmichFrame.Core/Logic/Pool/AllAssetsPool.cs +++ b/ImmichFrame.Core/Logic/Pool/AllAssetsPool.cs @@ -50,7 +50,7 @@ public async Task> GetAssets(int requested, Cancel var assets = await immichApi.SearchRandomAsync(searchDto, ct); - if (accountSettings.ExcludedAlbums.Any()) + if (accountSettings.ExcludedAlbums?.Any() ?? false) { var excludedAssetList = await GetExcludedAlbumAssets(ct); var excludedAssetSet = excludedAssetList.Select(x => x.Id).ToHashSet(); diff --git a/ImmichFrame.Core/Logic/Pool/PeopleAssetsPool.cs b/ImmichFrame.Core/Logic/Pool/PeopleAssetsPool.cs index fb3e42e9..b0a74b66 100644 --- a/ImmichFrame.Core/Logic/Pool/PeopleAssetsPool.cs +++ b/ImmichFrame.Core/Logic/Pool/PeopleAssetsPool.cs @@ -9,7 +9,13 @@ protected override async Task> LoadAssets(Cancella { var personAssets = new List(); - foreach (var personId in accountSettings.People) + var people = accountSettings.People; + if (people == null) + { + return personAssets; + } + + foreach (var personId in people) { int page = 1; int batchSize = 1000; diff --git a/ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs b/ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs index 4961555b..61225966 100644 --- a/ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs +++ b/ImmichFrame.Core/Logic/PooledImmichFrameLogic.cs @@ -35,7 +35,10 @@ private static TimeSpan RefreshInterval(int hours) private IAssetPool BuildPool(IAccountSettings accountSettings) { - if (!accountSettings.ShowFavorites && !accountSettings.ShowMemories && !accountSettings.Albums.Any() && !accountSettings.People.Any()) + var hasAlbums = accountSettings.Albums?.Any() ?? false; + var hasPeople = accountSettings.People?.Any() ?? false; + + if (!accountSettings.ShowFavorites && !accountSettings.ShowMemories && !hasAlbums && !hasPeople) { return new AllAssetsPool(_apiCache, _immichApi, accountSettings); } @@ -48,10 +51,10 @@ private IAssetPool BuildPool(IAccountSettings accountSettings) if (accountSettings.ShowMemories) pools.Add(new MemoryAssetsPool(_immichApi, accountSettings)); - if (accountSettings.Albums.Any()) + if (hasAlbums) pools.Add(new AlbumAssetsPool(_apiCache, _immichApi, accountSettings)); - if (accountSettings.People.Any()) + if (hasPeople) pools.Add(new PersonAssetsPool(_apiCache, _immichApi, accountSettings)); return new MultiAssetPool(pools);