Merge pull request #10865 from umbraco/v9/task/removed-VaritaionContextAccessor-from-IUmbracoContext

v9: Removed VariationContextAccessor from IUmbracoContext

src/Umbraco.Core/Web/IUmbracoContext.cs

@@ -1,5 +1,4 @@
 using System;
-using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Routing;
 
@@ -55,11 +54,6 @@ namespace Umbraco.Cms.Core.Web
         //// The only nicer way would be to have a RouteRequest method directly on IUmbracoContext but that means adding another dep to the ctx/factory.
         IPublishedRequest PublishedRequest { get; set; }
 
-        /// <summary>
-        /// Gets the variation context accessor.
-        /// </summary>
-        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
         /// </summary>

src/Umbraco.Infrastructure/Routing/ContentFinderByConfigured404.cs

@@ -21,6 +21,7 @@ namespace Umbraco.Cms.Core.Routing
         private readonly ContentSettings _contentSettings;
         private readonly IExamineManager _examineManager;
         private readonly IUmbracoContextAccessor _umbracoContextAccessor;
+        private readonly IVariationContextAccessor _variationContextAccessor;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ContentFinderByConfigured404"/> class.
@@ -30,12 +31,14 @@ namespace Umbraco.Cms.Core.Routing
             IEntityService entityService,
             IOptions<ContentSettings> contentConfigSettings,
             IExamineManager examineManager,
+            IVariationContextAccessor variationContextAccessor,
             IUmbracoContextAccessor umbracoContextAccessor)
         {
             _logger = logger;
             _entityService = entityService;
             _contentSettings = contentConfigSettings.Value;
             _examineManager = examineManager;
+            _variationContextAccessor = variationContextAccessor;
             _umbracoContextAccessor = umbracoContextAccessor;
         }
 
@@ -93,7 +96,7 @@ namespace Umbraco.Cms.Core.Routing
             var error404 = NotFoundHandlerHelper.GetCurrentNotFoundPageId(
                 _contentSettings.Error404Collection.ToArray(),
                 _entityService,
-                new PublishedContentQuery(umbCtx.PublishedSnapshot, umbCtx.VariationContextAccessor, _examineManager),
+                new PublishedContentQuery(umbCtx.PublishedSnapshot, _variationContextAccessor, _examineManager),
                 errorCulture,
                 domainContentId);
 

src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs

@@ -59,8 +59,6 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects
             var umbracoContextFactory = new UmbracoContextFactory(
                 umbracoContextAccessor,
                 snapshotService.Object,
-                new TestVariationContextAccessor(),
-                new TestDefaultCultureAccessor(),
                 new UmbracoRequestPaths(Options.Create(globalSettings), hostingEnvironment),
                 hostingEnvironment,
                 new UriUtility(hostingEnvironment),

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

@@ -3,7 +3,6 @@ using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Extensions;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Hosting;
-using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Routing;
 using Umbraco.Cms.Core.Web;
@@ -36,7 +35,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
             IPublishedSnapshotService publishedSnapshotService,
             UmbracoRequestPaths umbracoRequestPaths,
             IHostingEnvironment hostingEnvironment,
-            IVariationContextAccessor variationContextAccessor,
             UriUtility uriUtility,
             ICookieManager cookieManager,
             IHttpContextAccessor httpContextAccessor)
@@ -45,8 +43,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
             {
                 throw new ArgumentNullException(nameof(publishedSnapshotService));
             }
-
-            VariationContextAccessor = variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor));
             _uriUtility = uriUtility;
             _hostingEnvironment = hostingEnvironment;
             _cookieManager = cookieManager;
@@ -103,9 +99,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
         /// <inheritdoc/>
         public IPublishedRequest PublishedRequest { get; set; }
 
-        /// <inheritdoc/>
-        public IVariationContextAccessor VariationContextAccessor { get; }
-
         /// <inheritdoc/>
         public bool IsDebug => // NOTE: the request can be null during app startup!
                 _hostingEnvironment.IsDebugMode

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

@@ -2,7 +2,6 @@ using System;
 using Microsoft.AspNetCore.Http;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Hosting;
-using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Routing;
 using Umbraco.Cms.Core.Web;
@@ -16,8 +15,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
     {
         private readonly IUmbracoContextAccessor _umbracoContextAccessor;
         private readonly IPublishedSnapshotService _publishedSnapshotService;
-        private readonly IVariationContextAccessor _variationContextAccessor;
-        private readonly IDefaultCultureAccessor _defaultCultureAccessor;
         private readonly UmbracoRequestPaths _umbracoRequestPaths;
         private readonly IHostingEnvironment _hostingEnvironment;
         private readonly ICookieManager _cookieManager;
@@ -30,8 +27,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
         public UmbracoContextFactory(
             IUmbracoContextAccessor umbracoContextAccessor,
             IPublishedSnapshotService publishedSnapshotService,
-            IVariationContextAccessor variationContextAccessor,
-            IDefaultCultureAccessor defaultCultureAccessor,
             UmbracoRequestPaths umbracoRequestPaths,
             IHostingEnvironment hostingEnvironment,
             UriUtility uriUtility,
@@ -40,8 +35,6 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
         {
             _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor));
             _publishedSnapshotService = publishedSnapshotService ?? throw new ArgumentNullException(nameof(publishedSnapshotService));
-            _variationContextAccessor = variationContextAccessor ?? throw new ArgumentNullException(nameof(variationContextAccessor));
-            _defaultCultureAccessor = defaultCultureAccessor ?? throw new ArgumentNullException(nameof(defaultCultureAccessor));
             _umbracoRequestPaths = umbracoRequestPaths ?? throw new ArgumentNullException(nameof(umbracoRequestPaths));
             _hostingEnvironment = hostingEnvironment ?? throw new ArgumentNullException(nameof(hostingEnvironment));
             _uriUtility = uriUtility ?? throw new ArgumentNullException(nameof(uriUtility));
@@ -49,35 +42,13 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext
             _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor));
         }
 
-        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)
-            {
-                // TODO: By using _defaultCultureAccessor.DefaultCulture this means that the VariationContext will always return a variant culture, it will never
-                // return an empty string signifying that the culture is invariant. But does this matter? Are we actually expecting this to return an empty string
-                // for invariant routes? From what i can tell throughout the codebase is that whenever we are checking against the VariationContext.Culture we are
-                // also checking if the content type varies by culture or not. This is fine, however the code in the ctor of VariationContext is then misleading
-                // since it's assuming that the Culture can be empty (invariant) when in reality of a website this will never be empty since a real culture is always set here.
-                _variationContextAccessor.VariationContext = new VariationContext(_defaultCultureAccessor.DefaultCulture);
-            }
-
-            return new UmbracoContext(
+        private IUmbracoContext CreateUmbracoContext() => new UmbracoContext(
                 _publishedSnapshotService,
                 _umbracoRequestPaths,
                 _hostingEnvironment,
-                _variationContextAccessor,
                 _uriUtility,
                 _cookieManager,
                 _httpContextAccessor);
-        }
 
         /// <inheritdoc />
         public UmbracoContextReference EnsureUmbracoContext()