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

Fix collection creation when the collections library was just added

Original-merge: 3c698bab7fb7c607e726a0bf79f0bb0f7616e962

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

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

3 files changed, 78 insertions, 2 deletions

diff --git a/Emby.Server.Implementations/Collections/CollectionManager.cs b/Emby.Server.Implementations/Collections/CollectionManager.cs
index 84d50f5121..9567a29a98 100644
--- a/Emby.Server.Implementations/Collections/CollectionManager.cs
+++ b/Emby.Server.Implementations/Collections/CollectionManager.cs
@@ -114,7 +114,7 @@ namespace Emby.Server.Implementations.Collections
 
             _libraryManager.RootFolder.Children = null;
 
-            return FindFolders(path).First();
+            return FindFolders(path).FirstOrDefault();
         }
 
         internal string GetCollectionsFolderPath()
@@ -167,7 +167,7 @@ namespace Emby.Server.Implementations.Collections
 
             if (parentFolder is null)
             {
-                throw new ArgumentException(nameof(parentFolder));
+                throw new InvalidOperationException("Unable to resolve the collections library folder, so the collection cannot be created.");
             }
 
             var path = Path.Combine(parentFolder.Path, folderName);
diff --git a/MediaBrowser.Controller/Entities/AggregateFolder.cs b/MediaBrowser.Controller/Entities/AggregateFolder.cs
index a02802f41e..bb770fc6f3 100644
--- a/MediaBrowser.Controller/Entities/AggregateFolder.cs
+++ b/MediaBrowser.Controller/Entities/AggregateFolder.cs
@@ -54,6 +54,23 @@ namespace MediaBrowser.Controller.Entities
 
         public string[] PhysicalLocationsList { get; set; }
 
+        // Children caches the resolved items, _childrenIds the ids they were loaded from. Clearing
+        // only the former sends the next read back through LoadChildren, which replays the stale
+        // id list, so a caller invalidating this folder has to drop both.
+        public override IEnumerable<BaseItem> Children
+        {
+            get => base.Children;
+            set
+            {
+                if (value is null)
+                {
+                    ClearCache();
+                }
+
+                base.Children = value;
+            }
+        }
+
         public override bool CanDelete()
         {
             return false;
diff --git a/tests/Jellyfin.Controller.Tests/Entities/AggregateFolderTests.cs b/tests/Jellyfin.Controller.Tests/Entities/AggregateFolderTests.cs
new file mode 100644
index 0000000000..6c297673d4
--- /dev/null
+++ b/tests/Jellyfin.Controller.Tests/Entities/AggregateFolderTests.cs
@@ -0,0 +1,59 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using MediaBrowser.Controller.Entities;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Persistence;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Controller.Tests.Entities;
+
+public class AggregateFolderTests
+{
+    [Fact]
+    public void Children_ClearedAfterALibraryWasAdded_ListsTheNewLibrary()
+    {
+        var existing = new Folder { Id = Guid.NewGuid(), Path = "/libraries/movies" };
+        var added = new Folder { Id = Guid.NewGuid(), Path = "/libraries/collections" };
+
+        // What the repository holds grows once the new library has been resolved and stored.
+        var stored = new List<BaseItem> { existing };
+
+        var itemRepository = new Mock<IItemRepository>();
+        itemRepository.Setup(x => x.GetItemList(It.IsAny<InternalItemsQuery>()))
+            .Returns(() => stored.ToList());
+
+        var libraryManager = new Mock<ILibraryManager>();
+        libraryManager.Setup(x => x.GetItemById(It.IsAny<Guid>()))
+            .Returns((Guid id) => stored.Find(i => i.Id.Equals(id)));
+
+        BaseItem.ItemRepository = itemRepository.Object;
+        BaseItem.LibraryManager = libraryManager.Object;
+
+        var root = new AggregateFolder { Id = Guid.NewGuid(), Path = "/libraries" };
+
+        Assert.Equal([existing.Id], root.Children.Select(i => i.Id));
+
+        stored.Add(added);
+        root.Children = null;
+
+        // Null-forgiving: the setter takes null to mean "drop the cache", the getter reloads.
+        Assert.Equal([existing.Id, added.Id], root.Children!.Select(i => i.Id));
+    }
+
+    [Fact]
+    public void Children_AssignedASet_KeepsThatSet()
+    {
+        var itemRepository = new Mock<IItemRepository>(MockBehavior.Strict);
+        BaseItem.ItemRepository = itemRepository.Object;
+
+        var assigned = new Folder { Id = Guid.NewGuid(), Path = "/libraries/movies" };
+        var root = new AggregateFolder { Id = Guid.NewGuid(), Path = "/libraries" };
+
+        root.Children = [assigned];
+
+        // Never goes to the repository, so the strict mock stays unused.
+        Assert.Equal([assigned.Id], root.Children.Select(i => i.Id));
+    }
+}