Fixes DI Lifetime.Request works per thread. #6044

src/SolutionInfo.cs

@@ -18,5 +18,5 @@ using System.Resources;
 [assembly: AssemblyVersion("8.0.0")]
 
 // these are FYI and changed automatically
-[assembly: AssemblyFileVersion("8.1.2")]
-[assembly: AssemblyInformationalVersion("8.1.2")]
+[assembly: AssemblyFileVersion("8.1.1")]
+[assembly: AssemblyInformationalVersion("8.1.1")]

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/Migrations/Upgrade/V_8_0_0/DataTypes/DropDownFlexiblePreValueMigrator.cs

@@ -1,32 +0,0 @@
-using System.Collections.Generic;
-using System.Linq;
-using Umbraco.Core.PropertyEditors;
-
-namespace Umbraco.Core.Migrations.Upgrade.V_8_0_0.DataTypes
-{
-    class DropDownFlexiblePreValueMigrator : IPreValueMigrator
-    {
-        public bool CanMigrate(string editorAlias)
-            => editorAlias == "Umbraco.DropDown.Flexible";
-
-        public virtual string GetNewAlias(string editorAlias)
-            => null;
-
-        public object GetConfiguration(int dataTypeId, string editorAlias, Dictionary<string, PreValueDto> preValues)
-        {
-            var config = new DropDownFlexibleConfiguration();
-            foreach (var preValue in preValues.Values)
-            {
-                if (preValue.Alias == "multiple")
-                {
-                    config.Multiple = (preValue.Value == "1");
-                }
-                else
-                {
-                    config.Items.Add(new ValueListConfiguration.ValueListItem { Id = preValue.Id, Value = preValue.Value });
-                }
-            }
-            return config;
-        }
-    }
-}

src/Umbraco.Core/Migrations/Upgrade/V_8_0_0/DataTypes/PreValueMigratorComposer.cs

@@ -19,7 +19,6 @@ public class PreValueMigratorComposer : ICoreComposer
             .Append<NestedContentPreValueMigrator>()
             .Append<DecimalPreValueMigrator>()
             .Append<ListViewPreValueMigrator>()
-            .Append<DropDownFlexiblePreValueMigrator>()
             .Append<ValueListPreValueMigrator>();
     }
 }

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">
@@ -242,7 +242,6 @@
     <Compile Include="Migrations\Upgrade\V_8_0_0\DataTypes\DefaultPreValueMigrator.cs" />
     <Compile Include="Migrations\Upgrade\V_8_0_0\DataTypes\RenamingPreValueMigrator.cs" />
     <Compile Include="Migrations\Upgrade\V_8_0_0\DataTypes\RichTextPreValueMigrator.cs" />
-    <Compile Include="Migrations\Upgrade\V_8_0_0\DataTypes\DropDownFlexiblePreValueMigrator.cs" />
     <Compile Include="Migrations\Upgrade\V_8_0_0\MergeDateAndDateTimePropertyEditor.cs" />
     <Compile Include="Models\Entities\EntityExtensions.cs" />
     <Compile Include="Migrations\Upgrade\V_8_0_0\DataTypes\PreValueMigratorBase.cs" />

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.UI.Client/src/views/content/content.create.controller.js

@@ -94,11 +94,6 @@ function contentCreateController($scope,
         navigationService.hideDialog(showMenu);
     };
 
-    $scope.editContentType = function() {
-        $location.path("/settings/documenttypes/edit/" + $scope.contentTypeId).search("view", "permissions");
-        close();
-    }
-
     $scope.createBlank = createBlank;
     $scope.createOrSelectBlueprintIfAny = createOrSelectBlueprintIfAny;
     $scope.createFromBlueprint = createFromBlueprint;

src/Umbraco.Web.UI.Client/src/views/content/create.html

@@ -10,7 +10,7 @@
                 <p class="abstract" ng-if="!hasSettingsAccess"><localize key="create_noDocumentTypesWithNoSettingsAccess"/></p>
                 <div ng-if="hasSettingsAccess">
                     <p class="abstract"><localize key="create_noDocumentTypes" /></p>
-                    <button class="btn umb-outline" ng-click="editContentType()">
+                    <button class="btn umb-outline" href="#settings/documentTypes/edit/{{contentTypeId}}?view=permissions" ng-click="close()">
                         <localize key="create_noDocumentTypesEditPermissions"/>
                     </button>
                 </div>

src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker/mediapicker.controller.js

@@ -229,7 +229,7 @@ angular.module('umbraco').controller("Umbraco.PropertyEditors.MediaPickerControl
 
 
         $scope.sortableOptions = {
-            disabled: !multiPicker,
+            disabled: !$scope.isMultiPicker,
             items: "li:not(.add-wrapper)",
             cancel: ".unsortable",
             update: function (e, ui) {

src/Umbraco.Web.UI/Umbraco.Web.UI.csproj

@@ -345,9 +345,9 @@
         <WebProjectProperties>
           <UseIIS>False</UseIIS>
           <AutoAssignPort>True</AutoAssignPort>
-          <DevelopmentServerPort>8120</DevelopmentServerPort>
+          <DevelopmentServerPort>8110</DevelopmentServerPort>
           <DevelopmentServerVPath>/</DevelopmentServerVPath>
-          <IISUrl>http://localhost:8120</IISUrl>
+          <IISUrl>http://localhost:8110</IISUrl>
           <NTLMAuthentication>False</NTLMAuthentication>
           <UseCustomServer>False</UseCustomServer>
           <CustomServerUrl>

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" />