cleanup, notes

src/Umbraco.Core/Models/PublishedContent/HttpContextVariationContextAccessor.cs

@@ -1,4 +1,4 @@
-using Umbraco.Core.Cache;
+using Umbraco.Core.Cache;
 using Umbraco.Core.Models.PublishedContent;
 
 namespace Umbraco.Web.Models.PublishedContent
@@ -14,10 +14,7 @@ namespace Umbraco.Web.Models.PublishedContent
         /// <summary>
         /// Initializes a new instance of the <see cref="HttpContextVariationContextAccessor"/> class.
         /// </summary>
-        public HttpContextVariationContextAccessor(IRequestCache requestCache)
-        {
-            _requestCache = requestCache;
-        }
+        public HttpContextVariationContextAccessor(IRequestCache requestCache) => _requestCache = requestCache;
 
         /// <inheritdoc />
         public VariationContext VariationContext

src/Umbraco.Core/Routing/PublishedRouter.cs

@@ -98,6 +98,10 @@ namespace Umbraco.Web.Routing
 
         private void SetVariationContext(CultureInfo culture)
         {
+            // set the culture on the thread - once, so it's set when running document lookups
+            // TODO: Set this on HttpContext!
+            Thread.CurrentThread.CurrentUICulture = Thread.CurrentThread.CurrentCulture = culture;
+
             VariationContext variationContext = _variationContextAccessor.VariationContext;
             if (variationContext != null && variationContext.Culture == culture?.Name)
             {
@@ -124,9 +128,7 @@ namespace Umbraco.Web.Routing
                 return request.Build();
             }
 
-            // set the culture on the thread - once, so it's set when running document lookups
-            // TODO: Set this on HttpContext!
-            Thread.CurrentThread.CurrentUICulture = Thread.CurrentThread.CurrentCulture = request.Culture;
+            // set the culture
             SetVariationContext(request.Culture);
 
             // find the published content if it's not assigned. This could be manually assigned with a custom route handler, or
@@ -141,8 +143,7 @@ namespace Umbraco.Web.Routing
             // handle wildcard domains
             HandleWildcardDomains(request);
 
-            // set the culture on the thread -- again, 'cos it might have changed due to a finder or wildcard domain
-            Thread.CurrentThread.CurrentUICulture = Thread.CurrentThread.CurrentCulture = request.Culture;
+            // set the culture  -- again, 'cos it might have changed due to a finder or wildcard domain
             SetVariationContext(request.Culture);
 
             //// trigger the Prepared event - at that point it is still possible to change about anything
@@ -175,11 +176,9 @@ namespace Umbraco.Web.Routing
                 return frequest.Build();
             }
 
-            var result = frequest.Build();
+            IPublishedRequest result = frequest.Build();
 
-            // set the culture on the thread -- again, 'cos it might have changed in the event handler
-            // TODO: Set this on HttpContext!
-            Thread.CurrentThread.CurrentUICulture = Thread.CurrentThread.CurrentCulture = frequest.Culture;
+            // set the culture -- again, 'cos it might have changed in the event handler
             SetVariationContext(result.Culture);
 
             return result;

src/Umbraco.Core/UmbracoContextReference.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 
 namespace Umbraco.Web
 {
@@ -12,10 +12,10 @@ namespace Umbraco.Web
     /// it disposes the <see cref="UmbracoContext"/> and clears the
     /// <see cref="IUmbracoContextAccessor"/>.</para>
     /// </remarks>
-    public class UmbracoContextReference : IDisposable //fixme - should we inherit from DisposableObjectSlim?
+    public class UmbracoContextReference : IDisposable
     {
         private readonly IUmbracoContextAccessor _umbracoContextAccessor;
-        private bool _disposed;
+        private bool _disposedValue;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="UmbracoContextReference"/> class.
@@ -36,25 +36,25 @@ namespace Umbraco.Web
         /// <summary>
         /// Gets a value indicating whether the reference is a root reference.
         /// </summary>
-        /// <remarks>
-        /// <para></para>
-        /// </remarks>
         public bool IsRoot { get; }
 
-        /// <inheritdoc />
-        public void Dispose()
+        protected virtual void Dispose(bool disposing)
         {
-            if (_disposed)
-                return;
-            _disposed = true;
-
-            if (IsRoot)
+            if (!_disposedValue)
             {
-                UmbracoContext.Dispose();
-                _umbracoContextAccessor.UmbracoContext = null;
-            }
+                if (disposing)
+                {
+                    if (IsRoot)
+                    {
+                        UmbracoContext.Dispose();
+                        _umbracoContextAccessor.UmbracoContext = null;
+                    }
+                }
 
-            GC.SuppressFinalize(this);
+                _disposedValue = true;
+            }
         }
+
+        public void Dispose() => Dispose(disposing: true);
     }
 }

src/Umbraco.Core/Web/IUmbracoContext.cs

@@ -58,7 +58,7 @@ namespace Umbraco.Web
         /// <summary>
         /// Gets the variation context accessor.
         /// </summary>
-        IVariationContextAccessor VariationContextAccessor { get; } // TODO: Does this need to be a property, it can be injected when needed
+        IVariationContextAccessor VariationContextAccessor { get; } // TODO: This shouldn't expose the accessor should it? IUmbracoContext is basically the accessor to the VariationContext since IUmbracoContextFactory currently creates it?
 
         /// <summary>
         /// Gets a value indicating whether the request has debugging enabled

src/Umbraco.Infrastructure/Cache/DistributedCacheBinder.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;

src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs

@@ -1,17 +1,10 @@
 using System;
-using System.IO;
-using System.Text;
-using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Options;
-using Umbraco.Core.Configuration;
 using Umbraco.Core.Configuration.Models;
 using Umbraco.Core.Hosting;
 using Umbraco.Core.Models.PublishedContent;
 using Umbraco.Core.Security;
-using Umbraco.Core.Services;
-using Umbraco.Web.Common.Security;
 using Umbraco.Web.PublishedCache;
-using Umbraco.Web.Security;
 
 namespace Umbraco.Web
 {
@@ -61,6 +54,13 @@ namespace Umbraco.Web
 
         private IUmbracoContext CreateUmbracoContext()
         {
+            // TODO: It is strange having the IVariationContextAccessor initialized here and piggy backing off of IUmbracoContext.
+            // There's no particular reason that IVariationContextAccessor needs to exist as part of IUmbracoContext.
+            // Making this change however basically means that anywhere EnsureUmbracoContext is called, the IVariationContextAccessor
+            // would most likely need to be initialized too. This can easily happen in middleware for each request, however
+            // EnsureUmbracoContext is called for running on background threads too and it would be annoying to have to also ensure
+            // IVariationContextAccessor. Hrm.
+
             // make sure we have a variation context
             if (_variationContextAccessor.VariationContext == null)
             {

src/Umbraco.Web/UmbracoContextFactory.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.IO;
 using System.Text;
 using Umbraco.Core.Configuration;
@@ -11,6 +11,8 @@ using Umbraco.Web.Security;
 
 namespace Umbraco.Web
 {
+    // NOTE: This has been migrated to netcore
+
     /// <summary>
     /// Creates and manages <see cref="IUmbracoContext"/> instances.
     /// </summary>