Fixes DI Lifetime.Request works per thread. #6044

src/Umbraco.Core/Composing/Lifetime.cs

@@ -1,4 +1,6 @@
-namespace Umbraco.Core.Composing
+using System;
+
+namespace Umbraco.Core.Composing
 {
     /// <summary>
     /// Specifies the lifetime of a registered instance.
@@ -12,13 +14,6 @@
         /// or MS.DI, PerDependency in Autofac.</remarks>
         Transient,
 
-        // TODO: We need to fix this up, currently LightInject is the only one that behaves differently from all other containers.
-        //  ... the simple fix would be to map this to PerScopeLifetime in LI but need to wait on a response here https://github.com/seesharper/LightInject/issues/494#issuecomment-518942625
-        //
-        // we use it for controllers, httpContextBase and other request scoped objects: MembershpHelper, TagQuery, UmbracoTreeSearcher and ISearchableTree
-        // - so that they are automatically disposed at the end of the scope (ie request)
-        // - not sure they should not be simply 'scoped'?
-        
         /// <summary>
         /// One unique instance per request.
         /// </summary>
@@ -28,7 +23,9 @@
         /// </para>
         /// Corresponds to
         /// <para>
-        /// PerRequestLifeTime in LightInject - means transient but disposed at the end of the current web request.
+        /// PerScopeLifetime in LightInject.
+        /// Although it would seem that this should map to PerRequestLifeTime in LightInject,
+        /// that is actually misleading since PerRequestLifeTime in LightInject means transient but disposed at the end of the current web request.
         /// see: https://github.com/seesharper/LightInject/issues/494#issuecomment-518493262
         /// </para>
         /// <para>

src/Umbraco.Core/Composing/LightInject/LightInjectContainer.cs

@@ -145,7 +145,7 @@ namespace Umbraco.Core.Composing.LightInject
 
         /// <inheritdoc />
         public void Register(Type serviceType, Lifetime lifetime = Lifetime.Transient)
-            => Container.Register(serviceType, GetLifetime(lifetime));
+            => Container.Register(serviceType, GetLifetime(lifetime, serviceType));
 
         /// <inheritdoc />
         public void Register(Type serviceType, Type implementingType, Lifetime lifetime = Lifetime.Transient)
@@ -158,7 +158,7 @@ namespace Umbraco.Core.Composing.LightInject
                 case Lifetime.Request:
                 case Lifetime.Scope:
                 case Lifetime.Singleton:
-                    Container.Register(serviceType, implementingType, GetLifetime(lifetime));
+                    Container.Register(serviceType, implementingType, GetLifetime(lifetime, serviceType));
                     break;
                 default:
                     throw new NotSupportedException($"Lifetime {lifetime} is not supported.");
@@ -169,7 +169,7 @@ namespace Umbraco.Core.Composing.LightInject
         public void Register<TService>(Func<IFactory, TService> factory, Lifetime lifetime = Lifetime.Transient)
             where TService : class
         {
-            Container.Register(f => factory(this), GetLifetime(lifetime));
+            Container.Register(f => factory(this), GetLifetime(lifetime, typeof(TService)));
         }
 
         /// <inheritdoc />
@@ -186,7 +186,7 @@ namespace Umbraco.Core.Composing.LightInject
             where TService : class
         {
             // note that there can only be one implementation or instance registered "for" a service
-            Container.Register(typeof(TService), implementingType, GetTargetedServiceName<TTarget>(), GetLifetime(lifetime));
+            Container.Register(typeof(TService), implementingType, GetTargetedServiceName<TTarget>(), GetLifetime(lifetime, typeof(TService)));
         }
 
         /// <inheritdoc />
@@ -194,7 +194,7 @@ namespace Umbraco.Core.Composing.LightInject
             where TService : class
         {
             // note that there can only be one implementation or instance registered "for" a service
-            Container.Register(f => factory(this), GetTargetedServiceName<TTarget>(), GetLifetime(lifetime));
+            Container.Register(f => factory(this), GetTargetedServiceName<TTarget>(), GetLifetime(lifetime, typeof(TService)));
         }
 
         /// <inheritdoc />
@@ -202,14 +202,20 @@ namespace Umbraco.Core.Composing.LightInject
             where TService : class
             => Container.RegisterInstance(typeof(TService), instance, GetTargetedServiceName<TTarget>());
 
-        private ILifetime GetLifetime(Lifetime lifetime)
+        protected virtual ILifetime GetLifetime(Lifetime lifetime, Type type)
         {
             switch (lifetime)
             {
                 case Lifetime.Transient:
                     return null;
                 case Lifetime.Request:
-                    return new PerRequestLifeTime();
+                    //LightInject behaves slightly differently than all containers and although it would seem that we should be using
+                    //PerRequestLifeTime here, it actually does not mean "one per request", it
+                    //just means transient per request and will be disposed at the end of the request.
+                    //LightInject's PerScopeLifetime is equivalent to "one per request" which is what we'd expect when using the Lifetime.Request since
+                    //that would be consistent with other containers.
+                    //See: https://github.com/umbraco/Umbraco-CMS/issues/6044#issuecomment-518949758
+                    return new PerScopeLifetime();
                 case Lifetime.Scope:
                     return new PerScopeLifetime();
                 case Lifetime.Singleton:

src/Umbraco.Core/Umbraco.Core.csproj

@@ -71,7 +71,7 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
-    <PackageReference Include="LightInject" Version="5.4.0" />
+    <PackageReference Include="LightInject" Version="5.5.0" />
     <PackageReference Include="LightInject.Annotation" Version="1.1.0" />
     <PackageReference Include="LightInject.Web" Version="2.0.0" />
     <PackageReference Include="Microsoft.AspNet.Identity.Core">

src/Umbraco.Tests/Umbraco.Tests.csproj

@@ -83,7 +83,7 @@
     <PackageReference Include="HtmlAgilityPack">
       <Version>1.8.14</Version>
     </PackageReference>
-    <PackageReference Include="LightInject" Version="5.4.0" />
+    <PackageReference Include="LightInject" Version="5.5.0" />
     <PackageReference Include="LightInject.Annotation" Version="1.1.0" />
     <PackageReference Include="Lucene.Net" Version="3.0.3" />
     <PackageReference Include="Lucene.Net.Contrib" Version="3.0.3" />

src/Umbraco.Web/Composing/LightInject/LightInjectContainer.cs

@@ -1,6 +1,10 @@
 using System;
 using System.Web.Http;
+using System.Web.Http.Controllers;
+using System.Web.Mvc;
 using LightInject;
+using Umbraco.Core;
+using Umbraco.Core.Composing;
 using Umbraco.Core.Composing.LightInject;
 
 namespace Umbraco.Web.Composing.LightInject
@@ -34,5 +38,32 @@ namespace Umbraco.Web.Composing.LightInject
             Container.ScopeManagerProvider = smp; // reverts - we will do it last (in WebRuntime)
             Container.EnableWebApi(GlobalConfiguration.Configuration);
         }
+
+        protected override ILifetime GetLifetime(Lifetime lifetime, Type type)
+        {
+            switch (lifetime)
+            {
+                case Lifetime.Transient:
+                    return null;
+                case Lifetime.Request:
+                    //LightInject behaves slightly differently than all containers and based on feedback from the LightInject authors
+                    //it seems best to use PerRequestLifeTime for controllers even though controllers will work
+                    //just fine with PerScopeLifetime but this is just being 'safe'.
+                    //See: https://github.com/seesharper/LightInject/issues/494#issuecomment-519273614
+                    //Normally this will return PerScopeLifetime because in LightInject that means "one per request".
+                    //See: base class comments
+                    //See: https://github.com/umbraco/Umbraco-CMS/issues/6044#issuecomment-518949758
+
+                    return type.Inherits<IController>() || type.Inherits<IHttpController>()
+                        ? (ILifetime)new PerRequestLifeTime()
+                        : new PerScopeLifetime();
+                case Lifetime.Scope:
+                    return new PerScopeLifetime();
+                case Lifetime.Singleton:
+                    return new PerContainerLifetime();
+                default:
+                    throw new NotSupportedException($"Lifetime {lifetime} is not supported.");
+            }
+        }
     }
 }

src/Umbraco.Web/Umbraco.Web.csproj

@@ -67,7 +67,7 @@
     <PackageReference Include="ImageProcessor">
       <Version>2.7.0.100</Version>
     </PackageReference>
-    <PackageReference Include="LightInject" Version="5.4.0" />
+    <PackageReference Include="LightInject" Version="5.5.0" />
     <PackageReference Include="LightInject.Annotation" Version="1.1.0" />
     <PackageReference Include="LightInject.Mvc" Version="2.0.0" />
     <PackageReference Include="LightInject.WebApi" Version="2.0.0" />