Merge pull request #7719 from umbraco/v8/bugfix/fix-collection-items-DI

Fixes CollectionBuilder's to ensure that each item in the collectionis registered in DI with the same lifetime as the collection itself

src/Umbraco.Core/Composing/CollectionBuilderBase.cs

@@ -79,9 +79,12 @@ namespace Umbraco.Core.Composing
                 foreach (var type in types)
                     EnsureType(type, "register");
 
-                // register them
+                // register them - ensuring that each item is registered with the same lifetime as the collection.
+                // NOTE: Previously each one was not registered with the same lifetime which would mean that if there
+                // was a dependency on an individual item, it would resolve a brand new transient instance which isn't what
+                // we would expect to happen. The same item should be resolved from the container as the collection.
                 foreach (var type in types)
-                    register.Register(type);
+                    register.Register(type, CollectionLifetime);
 
                 _registeredTypes = types;
             }

src/Umbraco.Tests/Composing/CollectionBuildersTests.cs

@@ -354,7 +354,7 @@ namespace Umbraco.Tests.Composing
                 var col2 = factory.GetInstance<TestCollection>();
                 AssertCollection(col2, typeof(Resolved1), typeof(Resolved2));
 
-                AssertSameCollection(col1, col2);
+                AssertSameCollection(factory, col1, col2);
             }
 
         }
@@ -413,11 +413,11 @@ namespace Umbraco.Tests.Composing
             {
                 col1A = factory.GetInstance<TestCollection>();
                 col1B = factory.GetInstance<TestCollection>();
-            }
 
-            AssertCollection(col1A, typeof(Resolved1), typeof(Resolved2));
-            AssertCollection(col1B, typeof(Resolved1), typeof(Resolved2));
-            AssertSameCollection(col1A, col1B);
+                AssertCollection(col1A, typeof(Resolved1), typeof(Resolved2));
+                AssertCollection(col1B, typeof(Resolved1), typeof(Resolved2));
+                AssertSameCollection(factory, col1A, col1B);
+            }
 
             TestCollection col2;
 
@@ -452,7 +452,7 @@ namespace Umbraco.Tests.Composing
                 Assert.IsInstanceOf(expected[i], colA[i]);
         }
 
-        private static void AssertSameCollection(IEnumerable<Resolved> col1, IEnumerable<Resolved> col2)
+        private static void AssertSameCollection(IFactory factory, IEnumerable<Resolved> col1, IEnumerable<Resolved> col2)
         {
             Assert.AreSame(col1, col2);
 
@@ -460,8 +460,19 @@ namespace Umbraco.Tests.Composing
             var col2A = col2.ToArray();
 
             Assert.AreEqual(col1A.Length, col2A.Length);
+
+            // Ensure each item in each collection is the same but also
+            // resolve each item from the factory to ensure it's also the same since
+            // it should have the same lifespan.
             for (var i = 0; i < col1A.Length; i++)
+            {
                 Assert.AreSame(col1A[i], col2A[i]);
+
+                var itemA = factory.GetInstance(col1A[i].GetType());
+                var itemB = factory.GetInstance(col2A[i].GetType());
+
+                Assert.AreSame(itemA, itemB);
+            }
         }
 
         private static void AssertNotSameCollection(IEnumerable<Resolved> col1, IEnumerable<Resolved> col2)
@@ -472,8 +483,11 @@ namespace Umbraco.Tests.Composing
             var col2A = col2.ToArray();
 
             Assert.AreEqual(col1A.Length, col2A.Length);
+
             for (var i = 0; i < col1A.Length; i++)
+            {
                 Assert.AreNotSame(col1A[i], col2A[i]);
+            }   
         }
 
         #endregion