From 8d49fb3e8b85a1e2e9926e5a606835f0000a6eb4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 28 Nov 2018 16:19:50 +0100 Subject: [PATCH] Cleanup and document controller constructors --- .../Editors/AuthenticationController.cs | 32 +++--- .../Editors/DashboardController.cs | 10 +- src/Umbraco.Web/TagsController.cs | 16 +-- .../WebApi/UmbracoApiController.cs | 11 +- .../WebApi/UmbracoApiControllerBase.cs | 101 +++++++++--------- .../WebApi/UmbracoAuthorizedApiController.cs | 28 +++-- 6 files changed, 113 insertions(+), 85 deletions(-) diff --git a/src/Umbraco.Web/Editors/AuthenticationController.cs b/src/Umbraco.Web/Editors/AuthenticationController.cs index 3893afd62f..eee6997da2 100644 --- a/src/Umbraco.Web/Editors/AuthenticationController.cs +++ b/src/Umbraco.Web/Editors/AuthenticationController.cs @@ -40,27 +40,27 @@ namespace Umbraco.Web.Editors [IsBackOffice] public class AuthenticationController : UmbracoApiController { - - //fixme inject these private BackOfficeUserManager _userManager; private BackOfficeSignInManager _signInManager; - protected BackOfficeUserManager UserManager - { - get { return _userManager ?? (_userManager = TryGetOwinContext().Result.GetBackOfficeUserManager()); } - } - protected BackOfficeSignInManager SignInManager - { - get { return _signInManager ?? (_signInManager = TryGetOwinContext().Result.GetBackOfficeSignInManager()); } - } + /// + /// Initializes a new instance of the new class with auto dependencies. + /// public AuthenticationController() - { - } + { } - public AuthenticationController(IGlobalSettings globalSettings, UmbracoContext umbracoContext, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, ILogger logger, IProfilingLogger profilingLogger, IRuntimeState runtimeState) - : base(globalSettings, umbracoContext, sqlContext, services, applicationCache, logger, profilingLogger, runtimeState) - { - } + /// + /// Initializes a new instance of the class with all its dependencies. + /// + public AuthenticationController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, IProfilingLogger logger, IRuntimeState runtimeState) + : base(globalSettings, umbracoContextAccessor, sqlContext, services, applicationCache, logger, runtimeState) + { } + + protected BackOfficeUserManager UserManager => _userManager + ?? (_userManager = TryGetOwinContext().Result.GetBackOfficeUserManager()); + + protected BackOfficeSignInManager SignInManager => _signInManager + ?? (_signInManager = TryGetOwinContext().Result.GetBackOfficeSignInManager()); /// /// Returns the configuration for the backoffice user membership provider - used to configure the change password dialog diff --git a/src/Umbraco.Web/Editors/DashboardController.cs b/src/Umbraco.Web/Editors/DashboardController.cs index 8a8db06d8d..b8a961ee5a 100644 --- a/src/Umbraco.Web/Editors/DashboardController.cs +++ b/src/Umbraco.Web/Editors/DashboardController.cs @@ -27,11 +27,17 @@ namespace Umbraco.Web.Editors [WebApi.UmbracoAuthorize] public class DashboardController : UmbracoApiController { + /// + /// Initializes a new instance of the with auto dependencies. + /// public DashboardController() { } - public DashboardController(IGlobalSettings globalSettings, UmbracoContext umbracoContext, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, ILogger logger, IProfilingLogger profilingLogger, IRuntimeState runtimeState) - : base(globalSettings, umbracoContext, sqlContext, services, applicationCache, logger, profilingLogger, runtimeState) + /// + /// Initializes a new instance of the with all its dependencies. + /// + public DashboardController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, IProfilingLogger logger, IRuntimeState runtimeState) + : base(globalSettings, umbracoContextAccessor, sqlContext, services, applicationCache, logger, runtimeState) { } //we have just one instance of HttpClient shared for the entire application diff --git a/src/Umbraco.Web/TagsController.cs b/src/Umbraco.Web/TagsController.cs index a85b37c545..181b9f7da2 100644 --- a/src/Umbraco.Web/TagsController.cs +++ b/src/Umbraco.Web/TagsController.cs @@ -20,14 +20,18 @@ namespace Umbraco.Web.WebServices // TODO: This controller should be moved to a more suitable place. public class TagsController : UmbracoApiController { + /// + /// Initializes a new instance of the with auto dependencies. + /// public TagsController() - { - } + { } - public TagsController(IGlobalSettings globalSettings, UmbracoContext umbracoContext, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, ILogger logger, IProfilingLogger profilingLogger, IRuntimeState runtimeState) - : base(globalSettings, umbracoContext, sqlContext, services, applicationCache, logger, profilingLogger, runtimeState) - { - } + /// + /// Initializes a new instance of the with all its dependencies. + /// + public TagsController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, IProfilingLogger logger, IRuntimeState runtimeState) + : base(globalSettings, umbracoContextAccessor, sqlContext, services, applicationCache, logger, runtimeState) + { } /// /// Get every tag stored in the database (with optional group) diff --git a/src/Umbraco.Web/WebApi/UmbracoApiController.cs b/src/Umbraco.Web/WebApi/UmbracoApiController.cs index 4f2934abf4..3db3610cc2 100644 --- a/src/Umbraco.Web/WebApi/UmbracoApiController.cs +++ b/src/Umbraco.Web/WebApi/UmbracoApiController.cs @@ -13,11 +13,18 @@ namespace Umbraco.Web.WebApi /// public abstract class UmbracoApiController : UmbracoApiControllerBase, IDiscoverable { + /// + /// Initializes a new instance of the with auto dependencies. + /// + /// Dependencies are obtained from the service locator. protected UmbracoApiController() { } - protected UmbracoApiController(IGlobalSettings globalSettings, UmbracoContext umbracoContext, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, ILogger logger, IProfilingLogger profilingLogger, IRuntimeState runtimeState) - : base(globalSettings, umbracoContext, sqlContext, services, applicationCache, logger, profilingLogger, runtimeState) + /// + /// Initialize a new instance of the with all its dependencies. + /// + protected UmbracoApiController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, IProfilingLogger logger, IRuntimeState runtimeState) + : base(globalSettings, umbracoContextAccessor, sqlContext, services, applicationCache, logger, runtimeState) { } } } diff --git a/src/Umbraco.Web/WebApi/UmbracoApiControllerBase.cs b/src/Umbraco.Web/WebApi/UmbracoApiControllerBase.cs index 27b06ee767..4b23d4f0a8 100644 --- a/src/Umbraco.Web/WebApi/UmbracoApiControllerBase.cs +++ b/src/Umbraco.Web/WebApi/UmbracoApiControllerBase.cs @@ -21,48 +21,82 @@ namespace Umbraco.Web.WebApi [FeatureAuthorize] public abstract class UmbracoApiControllerBase : ApiController { + private readonly IUmbracoContextAccessor _umbracoContextAccessor; private UmbracoHelper _umbracoHelper; - // for debugging purposes + // note: all Umbraco controllers have two constructors: one with all dependencies, which should be used, + // and one with auto dependencies, ie no dependencies - and then dependencies are automatically obtained + // here from the Current service locator - this is obviously evil, but it allows us to add new dependencies + // without breaking compatibility. + + /// + /// Initializes a new instance of the class with auto dependencies. + /// + /// Dependencies are obtained from the service locator. + protected UmbracoApiControllerBase() + : this( + Current.Factory.GetInstance(), + Current.Factory.GetInstance(), + Current.Factory.GetInstance(), + Current.Factory.GetInstance(), + Current.Factory.GetInstance(), + Current.Factory.GetInstance(), + Current.Factory.GetInstance() + ) + { } + + /// + /// Initializes a new instance of the class with all its dependencies. + /// + protected UmbracoApiControllerBase(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, IProfilingLogger logger, IRuntimeState runtimeState) + { + GlobalSettings = globalSettings; + _umbracoContextAccessor = umbracoContextAccessor; + SqlContext = sqlContext; + Services = services; + ApplicationCache = applicationCache; + Logger = logger; + RuntimeState = runtimeState; + } + + /// + /// Gets a unique instance identifier. + /// + /// For debugging purposes. internal Guid InstanceId { get; } = Guid.NewGuid(); /// - /// Gets or sets the Umbraco context. + /// Gets the Umbraco context. /// public virtual IGlobalSettings GlobalSettings { get; } /// - /// Gets or sets the Umbraco context. + /// Gets the Umbraco context. /// - public virtual UmbracoContext UmbracoContext { get; } + public virtual UmbracoContext UmbracoContext => _umbracoContextAccessor.UmbracoContext; /// - /// Gets or sets the sql context. + /// Gets the sql context. /// public ISqlContext SqlContext { get; } /// - /// Gets or sets the services context. + /// Gets the services context. /// public ServiceContext Services { get; } /// - /// Gets or sets the application cache. + /// Gets the application cache. /// public CacheHelper ApplicationCache { get; } /// - /// Gets or sets the logger. + /// Gets the logger. /// - public ILogger Logger { get; } + public IProfilingLogger Logger { get; } /// - /// Gets or sets the profiling logger. - /// - public IProfilingLogger ProfilingLogger { get; } - - /// - /// Gets or sets the runtime state. + /// Gets the runtime state. /// internal IRuntimeState RuntimeState { get; } @@ -87,49 +121,16 @@ namespace Umbraco.Web.WebApi /// public WebSecurity Security => UmbracoContext.Security; - protected UmbracoApiControllerBase() - : this( - Current.Factory.GetInstance(), - Current.Factory.GetInstance().UmbracoContext, - Current.Factory.GetInstance(), - Current.Factory.GetInstance(), - Current.Factory.GetInstance(), - Current.Factory.GetInstance(), - Current.Factory.GetInstance(), - Current.Factory.GetInstance() - ) - { - } - - // fixme - Inject fewer things? (Aggregate more) - // fixme - inject the context accessor not the context itself? - // fixme - profiling logger is logger, merge! - protected UmbracoApiControllerBase(IGlobalSettings globalSettings, UmbracoContext umbracoContext, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, ILogger logger, IProfilingLogger profilingLogger, IRuntimeState runtimeState) - { - GlobalSettings = globalSettings; - UmbracoContext = umbracoContext; - SqlContext = sqlContext; - Services = services; - ApplicationCache = applicationCache; - Logger = logger; - ProfilingLogger = profilingLogger; - RuntimeState = runtimeState; - } - /// /// Tries to get the current HttpContext. /// protected Attempt TryGetHttpContext() - { - return Request.TryGetHttpContext(); - } + => Request.TryGetHttpContext(); /// /// Tries to get the current OWIN context. /// protected Attempt TryGetOwinContext() - { - return Request.TryGetOwinContext(); - } + => Request.TryGetOwinContext(); } } diff --git a/src/Umbraco.Web/WebApi/UmbracoAuthorizedApiController.cs b/src/Umbraco.Web/WebApi/UmbracoAuthorizedApiController.cs index 82c172af79..4ae9c00a47 100644 --- a/src/Umbraco.Web/WebApi/UmbracoAuthorizedApiController.cs +++ b/src/Umbraco.Web/WebApi/UmbracoAuthorizedApiController.cs @@ -5,18 +5,18 @@ using Umbraco.Core.Logging; using Umbraco.Web.WebApi.Filters; using Umbraco.Core.Models.Identity; using Umbraco.Core.Persistence; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Security; namespace Umbraco.Web.WebApi { /// - /// Provides a base class for autorized auto-routed Umbraco API controllers. + /// Provides a base class for authorized auto-routed Umbraco API controllers. /// /// - /// This controller will also append a custom header to the response if the user is logged in using forms authentication - /// which indicates the seconds remaining before their timeout expires. + /// This controller will also append a custom header to the response if the user + /// is logged in using forms authentication which indicates the seconds remaining + /// before their timeout expires. /// [IsBackOffice] [UmbracoUserTimeoutFilter] @@ -30,14 +30,24 @@ namespace Umbraco.Web.WebApi { private BackOfficeUserManager _userManager; - protected BackOfficeUserManager UserManager - => _userManager ?? (_userManager = TryGetOwinContext().Result.GetBackOfficeUserManager()); - + /// + /// Initializes a new instance of the with auto dependencies. + /// + /// Dependencies are obtained from the service locator. protected UmbracoAuthorizedApiController() { } - protected UmbracoAuthorizedApiController(IGlobalSettings globalSettings, UmbracoContext umbracoContext, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, ILogger logger, IProfilingLogger profilingLogger, IRuntimeState runtimeState) - : base(globalSettings, umbracoContext, sqlContext, services, applicationCache, logger, profilingLogger, runtimeState) + /// + /// Initializes a new instance of the class with all its dependencies. + /// + protected UmbracoAuthorizedApiController(IGlobalSettings globalSettings, IUmbracoContextAccessor umbracoContextAccessor, ISqlContext sqlContext, ServiceContext services, CacheHelper applicationCache, IProfilingLogger logger, IRuntimeState runtimeState) + : base(globalSettings, umbracoContextAccessor, sqlContext, services, applicationCache, logger, runtimeState) { } + + /// + /// Gets the user manager. + /// + protected BackOfficeUserManager UserManager + => _userManager ?? (_userManager = TryGetOwinContext().Result.GetBackOfficeUserManager()); } }