From 8c33a31f378f5d9a22d887b246826d9eb07780a6 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 30 May 2016 17:56:25 +0200 Subject: [PATCH 1/2] U4-8447 - fix services setup --- .../Security/BackOfficeUserManager.cs | 6 +- .../Security/BackOfficeUserStore.cs | 6 +- src/Umbraco.Core/Services/ContentService.cs | 17 +--- .../Services/ContentTypeService.cs | 23 +----- .../Services/IMembershipMemberService.cs | 8 -- src/Umbraco.Core/Services/MediaService.cs | 17 +--- src/Umbraco.Core/Services/MediaTypeService.cs | 20 +---- src/Umbraco.Core/Services/MemberService.cs | 38 ++++----- .../Services/MemberTypeService.cs | 20 +---- .../UmbracoServiceMembershipProviderTests.cs | 81 ++++++++++--------- .../config/ClientDependency.config | 2 +- .../Security/Identity/AppBuilderExtensions.cs | 1 + .../Providers/MembersMembershipProvider.cs | 8 +- .../Providers/UsersMembershipProvider.cs | 8 +- 14 files changed, 90 insertions(+), 165 deletions(-) diff --git a/src/Umbraco.Core/Security/BackOfficeUserManager.cs b/src/Umbraco.Core/Security/BackOfficeUserManager.cs index e48079c10b..69be7b60c2 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserManager.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserManager.cs @@ -32,25 +32,29 @@ namespace Umbraco.Core.Security } #region Static Create methods + /// /// Creates a BackOfficeUserManager instance with all default options and the default BackOfficeUserManager /// /// /// + /// /// /// /// public static BackOfficeUserManager Create( IdentityFactoryOptions options, IUserService userService, + IMemberTypeService memberTypeService, IExternalLoginService externalLoginService, MembershipProviderBase membershipProvider) { if (options == null) throw new ArgumentNullException("options"); if (userService == null) throw new ArgumentNullException("userService"); + if (memberTypeService == null) throw new ArgumentNullException("memberTypeService"); if (externalLoginService == null) throw new ArgumentNullException("externalLoginService"); - var manager = new BackOfficeUserManager(new BackOfficeUserStore(userService, externalLoginService, membershipProvider)); + var manager = new BackOfficeUserManager(new BackOfficeUserStore(userService, memberTypeService, externalLoginService, membershipProvider)); manager.InitUserManager(manager, membershipProvider, options); return manager; } diff --git a/src/Umbraco.Core/Security/BackOfficeUserStore.cs b/src/Umbraco.Core/Security/BackOfficeUserStore.cs index c6714e256a..40c532976e 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserStore.cs @@ -30,12 +30,14 @@ namespace Umbraco.Core.Security //IQueryableUserStore { private readonly IUserService _userService; + private readonly IMemberTypeService _memberTypeService; private readonly IExternalLoginService _externalLoginService; private bool _disposed = false; - public BackOfficeUserStore(IUserService userService, IExternalLoginService externalLoginService, MembershipProviderBase usersMembershipProvider) + public BackOfficeUserStore(IUserService userService, IMemberTypeService memberTypeService, IExternalLoginService externalLoginService, MembershipProviderBase usersMembershipProvider) { _userService = userService; + _memberTypeService = memberTypeService; _externalLoginService = externalLoginService; if (userService == null) throw new ArgumentNullException("userService"); if (usersMembershipProvider == null) throw new ArgumentNullException("usersMembershipProvider"); @@ -69,7 +71,7 @@ namespace Umbraco.Core.Security if (user == null) throw new ArgumentNullException("user"); var userType = _userService.GetUserTypeByAlias( - user.UserTypeAlias.IsNullOrWhiteSpace() ? _userService.GetDefaultMemberType() : user.UserTypeAlias); + user.UserTypeAlias.IsNullOrWhiteSpace() ? _memberTypeService.GetDefault() : user.UserTypeAlias); var member = new User(userType) { diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index c2f803e945..1b19f57c73 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -25,7 +25,6 @@ namespace Umbraco.Core.Services private readonly IDataTypeService _dataTypeService; private readonly IUserService _userService; private readonly IEnumerable _urlSegmentProviders; - private IContentTypeService _contentTypeService; #region Constructors @@ -46,20 +45,6 @@ namespace Umbraco.Core.Services _urlSegmentProviders = urlSegmentProviders; } - // don't change or remove this, will need it later - private IContentTypeService ContentTypeService => _contentTypeService; - //// handle circular dependencies - //internal IContentTypeService ContentTypeService - //{ - // get - // { - // if (_contentTypeService == null) - // throw new InvalidOperationException("ContentService.ContentTypeService has not been initialized."); - // return _contentTypeService; - // } - // set { _contentTypeService = value; } - //} - #endregion #region Count @@ -2581,7 +2566,7 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.CreateUnitOfWork()) { - uow.ReadLock(Constants.Locks.ContentTree); + uow.ReadLock(Constants.Locks.ContentTypes); var repository = uow.CreateRepository(); var query = repository.Query.Where(x => x.Alias == contentTypeAlias); diff --git a/src/Umbraco.Core/Services/ContentTypeService.cs b/src/Umbraco.Core/Services/ContentTypeService.cs index fcbad641cf..d007c38bf4 100644 --- a/src/Umbraco.Core/Services/ContentTypeService.cs +++ b/src/Umbraco.Core/Services/ContentTypeService.cs @@ -17,32 +17,17 @@ namespace Umbraco.Core.Services /// internal class ContentTypeService : ContentTypeServiceBase, IContentTypeService { - private IContentService _contentService; - public ContentTypeService(IDatabaseUnitOfWorkProvider provider, ILogger logger, IEventMessagesFactory eventMessagesFactory, IContentService contentService) : base(provider, logger, eventMessagesFactory) { - _contentService = contentService; + ContentService = contentService; } - // beware! order is important to avoid deadlocks protected override int[] ReadLockIds { get; } = { Constants.Locks.ContentTypes }; protected override int[] WriteLockIds { get; } = { Constants.Locks.ContentTree, Constants.Locks.ContentTypes }; - // don't change or remove this, will need it later - private IContentService ContentService => _contentService; - //// handle circular dependencies - //internal IContentService ContentService - //{ - // get - // { - // if (_contentService == null) - // throw new InvalidOperationException("ContentTypeService.ContentService has not been initialized."); - // return _contentService; - // } - // set { _contentService = value; } - //} + private IContentService ContentService { get; } protected override Guid ContainedObjectType => Constants.ObjectTypes.DocumentTypeGuid; @@ -141,7 +126,7 @@ namespace Umbraco.Core.Services var toUpdate = GetContentTypesForXmlUpdates(contentTypes).ToArray(); if (toUpdate.Any() == false) return; - var contentService = _contentService as ContentService; + var contentService = ContentService as ContentService; if (contentService != null) { contentService.RePublishAll(toUpdate.Select(x => x.Id).ToArray()); @@ -149,7 +134,7 @@ namespace Umbraco.Core.Services else { //this should never occur, the content service should always be typed but we'll check anyways. - _contentService.RePublishAll(); + ContentService.RePublishAll(); } } } diff --git a/src/Umbraco.Core/Services/IMembershipMemberService.cs b/src/Umbraco.Core/Services/IMembershipMemberService.cs index 4698ffd966..8a64198b9e 100644 --- a/src/Umbraco.Core/Services/IMembershipMemberService.cs +++ b/src/Umbraco.Core/Services/IMembershipMemberService.cs @@ -46,14 +46,6 @@ namespace Umbraco.Core.Services /// with number of Members or Users for passed in type int GetCount(MemberCountType countType); - /// - /// Gets the default MemberType alias - /// - /// By default we'll return the 'writer', but we need to check it exists. If it doesn't we'll - /// return the first type that is not an admin, otherwise if there's only one we will return that one. - /// Alias of the default MemberType - string GetDefaultMemberType(); - /// /// Checks if a Member with the username exists /// diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 79f52d7b5c..0355378ed0 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -24,7 +24,6 @@ namespace Umbraco.Core.Services private readonly IDataTypeService _dataTypeService; private readonly IUserService _userService; private readonly IEnumerable _urlSegmentProviders; - private IMediaTypeService _mediaTypeService; #region Constructors @@ -45,20 +44,6 @@ namespace Umbraco.Core.Services _urlSegmentProviders = urlSegmentProviders; } - // don't change or remove this, will need it later - private IMediaTypeService MediaTypeService => _mediaTypeService; - //// handle circular dependencies - //internal IMediaTypeService MediaTypeService - //{ - // get - // { - // if (_mediaTypeService == null) - // throw new InvalidOperationException("MediaService.MediaTypeService has not been initialized."); - // return _mediaTypeService; - // } - // set { _mediaTypeService = value; } - //} - #endregion #region Count @@ -1373,7 +1358,7 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.CreateUnitOfWork()) { - uow.ReadLock(Constants.Locks.MediaTree); + uow.ReadLock(Constants.Locks.MediaTypes); var repository = uow.CreateRepository(); var query = repository.Query.Where(x => x.Alias == mediaTypeAlias); diff --git a/src/Umbraco.Core/Services/MediaTypeService.cs b/src/Umbraco.Core/Services/MediaTypeService.cs index 69f26c52f2..d68794cc92 100644 --- a/src/Umbraco.Core/Services/MediaTypeService.cs +++ b/src/Umbraco.Core/Services/MediaTypeService.cs @@ -11,31 +11,17 @@ namespace Umbraco.Core.Services { internal class MediaTypeService : ContentTypeServiceBase, IMediaTypeService { - private IMediaService _mediaService; - public MediaTypeService(IDatabaseUnitOfWorkProvider provider, ILogger logger, IEventMessagesFactory eventMessagesFactory, IMediaService mediaService) : base(provider, logger, eventMessagesFactory) { - _mediaService = mediaService; + MediaService = mediaService; } // beware! order is important to avoid deadlocks protected override int[] ReadLockIds { get; } = { Constants.Locks.MediaTypes }; protected override int[] WriteLockIds { get; } = { Constants.Locks.MediaTree, Constants.Locks.MediaTypes }; - // don't remove, will need it later - private IMediaService MediaService => _mediaService; - //// handle circular dependencies - //internal IMediaService MediaService - //{ - // get - // { - // if (_mediaService == null) - // throw new InvalidOperationException("MediaTypeService.MediaService has not been initialized."); - // return _mediaService; - // } - // set { _mediaService = value; } - //} + private IMediaService MediaService { get; } protected override Guid ContainedObjectType => Constants.ObjectTypes.MediaTypeGuid; @@ -50,7 +36,7 @@ namespace Umbraco.Core.Services var toUpdate = GetContentTypesForXmlUpdates(contentTypes).ToArray(); if (toUpdate.Any() == false) return; - var mediaService = _mediaService as MediaService; + var mediaService = MediaService as MediaService; mediaService?.RebuildXmlStructures(toUpdate.Select(x => x.Id).ToArray()); } } diff --git a/src/Umbraco.Core/Services/MemberService.cs b/src/Umbraco.Core/Services/MemberService.cs index 46aea609da..78f6d5a523 100644 --- a/src/Umbraco.Core/Services/MemberService.cs +++ b/src/Umbraco.Core/Services/MemberService.cs @@ -24,7 +24,6 @@ namespace Umbraco.Core.Services private readonly EntityXmlSerializer _entitySerializer = new EntityXmlSerializer(); private readonly IDataTypeService _dataTypeService; private readonly IMemberGroupService _memberGroupService; - private IMemberTypeService _memberTypeService; #region Constructor @@ -42,20 +41,6 @@ namespace Umbraco.Core.Services _dataTypeService = dataTypeService; } - // don't change or remove this, will need it later - private IMemberTypeService MemberTypeService => _memberTypeService; - //// handle circular dependencies - //internal IMemberTypeService MemberTypeService - //{ - // get - // { - // if (_memberTypeService == null) - // throw new InvalidOperationException("MemberService.MemberTypeService has not been initialized."); - // return _memberTypeService; - // } - // set { _memberTypeService = value; } - //} - #endregion #region Count @@ -1356,16 +1341,21 @@ namespace Umbraco.Core.Services private IMemberType GetMemberType(string memberTypeAlias) { - var memberType = MemberTypeService.Get(memberTypeAlias); - if (memberType == null) - throw new Exception(string.Format("No MemberType matching alias: \"{0}\".", memberTypeAlias)); - return memberType; - } + Mandate.ParameterNotNullOrEmpty(memberTypeAlias, nameof(memberTypeAlias)); - [Obsolete("use MemberTypeService.GetDefault()")] // fixme kill! - public string GetDefaultMemberType() - { - return MemberTypeService.GetDefault(); + using (var uow = UowProvider.CreateUnitOfWork()) + { + uow.ReadLock(Constants.Locks.MemberTypes); + + var repository = uow.CreateRepository(); + var memberType = repository.Get(memberTypeAlias); + + if (memberType == null) + throw new Exception($"No MemberType matching the passed in Alias: '{memberTypeAlias}' was found"); // causes rollback + + uow.Complete(); + return memberType; + } } #endregion diff --git a/src/Umbraco.Core/Services/MemberTypeService.cs b/src/Umbraco.Core/Services/MemberTypeService.cs index df78b59bd1..8376bf19b2 100644 --- a/src/Umbraco.Core/Services/MemberTypeService.cs +++ b/src/Umbraco.Core/Services/MemberTypeService.cs @@ -11,31 +11,17 @@ namespace Umbraco.Core.Services { internal class MemberTypeService : ContentTypeServiceBase, IMemberTypeService { - private IMemberService _memberService; - public MemberTypeService(IDatabaseUnitOfWorkProvider provider, ILogger logger, IEventMessagesFactory eventMessagesFactory, IMemberService memberService) : base(provider, logger, eventMessagesFactory) { - _memberService = memberService; + MemberService = memberService; } // beware! order is important to avoid deadlocks protected override int[] ReadLockIds { get; } = { Constants.Locks.MemberTypes }; protected override int[] WriteLockIds { get; } = { Constants.Locks.MemberTree, Constants.Locks.MemberTypes }; - // don't remove, will need it later - private IMemberService MemberService => _memberService; - //// handle circular dependencies - //internal IMemberService MemberService - //{ - // get - // { - // if (_memberService == null) - // throw new InvalidOperationException("MemberTypeService.MemberService has not been initialized."); - // return _memberService; - // } - // set { _memberService = value; } - //} + private IMemberService MemberService { get; } protected override Guid ContainedObjectType => Constants.ObjectTypes.MemberTypeGuid; @@ -51,7 +37,7 @@ namespace Umbraco.Core.Services var toUpdate = GetContentTypesForXmlUpdates(contentTypes).ToArray(); if (toUpdate.Any() == false) return; - var memberService = _memberService as MemberService; + var memberService = MemberService as MemberService; memberService?.RebuildXmlStructures(toUpdate.Select(x => x.Id).ToArray()); } diff --git a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs index 72900351b2..f0525ff55b 100644 --- a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs +++ b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs @@ -18,20 +18,20 @@ namespace Umbraco.Tests.Membership [Test] public void Sets_Default_Member_Type_From_Service_On_Init() { - var mServiceMock = new Mock(); - var provider = new MembersMembershipProvider(mServiceMock.Object); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Blah"); + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Blah"); + var provider = new MembersMembershipProvider(Mock.Of(), memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection()); - + Assert.AreEqual("Blah", provider.DefaultMemberTypeAlias); } [Test] public void Sets_Default_Member_Type_From_Config_On_Init() { - var mServiceMock = new Mock(); - var provider = new MembersMembershipProvider(mServiceMock.Object); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Blah"); + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Blah"); + var provider = new MembersMembershipProvider(Mock.Of(), memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection { { "defaultMemberTypeAlias", "Hello" } }); Assert.AreEqual("Hello", provider.DefaultMemberTypeAlias); @@ -40,11 +40,12 @@ namespace Umbraco.Tests.Membership [Test] public void Create_User_Already_Exists() { - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.Exists("test")).Returns(true); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Member"); + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Member"); + var membershipServiceMock = new Mock(); + membershipServiceMock.Setup(service => service.Exists("test")).Returns(true); - var provider = new MembersMembershipProvider(mServiceMock.Object); + var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection()); MembershipCreateStatus status; @@ -56,11 +57,12 @@ namespace Umbraco.Tests.Membership [Test] public void Create_User_Requires_Unique_Email() { - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => new Member("test", MockedContentTypes.CreateSimpleMemberType())); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Member"); + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Member"); + var membershipServiceMock = new Mock(); + membershipServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => new Member("test", MockedContentTypes.CreateSimpleMemberType())); - var provider = new MembersMembershipProvider(mServiceMock.Object); + var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection { { "requiresUniqueEmail", "true" } }); MembershipCreateStatus status; @@ -78,20 +80,21 @@ namespace Umbraco.Tests.Membership { memberType.AddPropertyType(p.Value); } - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.Exists("test")).Returns(false); - mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Member"); - mServiceMock.Setup( + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Member"); + var membershipServiceMock = new Mock(); + membershipServiceMock.Setup(service => service.Exists("test")).Returns(false); + membershipServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); + membershipServiceMock.Setup( service => service.CreateWithIdentity(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Callback((string u, string e, string p, string m) => { createdMember = new Member("test", e, u, p, memberType); }) .Returns(() => createdMember); - var provider = new MembersMembershipProvider(mServiceMock.Object); + var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection()); - + MembershipCreateStatus status; provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); @@ -109,11 +112,12 @@ namespace Umbraco.Tests.Membership { memberType.AddPropertyType(p.Value); } - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.Exists("test")).Returns(false); - mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Member"); - mServiceMock.Setup( + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Member"); + var membershipServiceMock = new Mock(); + membershipServiceMock.Setup(service => service.Exists("test")).Returns(false); + membershipServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); + membershipServiceMock.Setup( service => service.CreateWithIdentity(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Callback((string u, string e, string p, string m) => { @@ -121,9 +125,9 @@ namespace Umbraco.Tests.Membership }) .Returns(() => createdMember); - var provider = new MembersMembershipProvider(mServiceMock.Object); + var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); - + MembershipCreateStatus status; provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); @@ -142,11 +146,12 @@ namespace Umbraco.Tests.Membership { memberType.AddPropertyType(p.Value); } - var mServiceMock = new Mock(); - mServiceMock.Setup(service => service.Exists("test")).Returns(false); - mServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); - mServiceMock.Setup(service => service.GetDefaultMemberType()).Returns("Member"); - mServiceMock.Setup( + var memberTypeServiceMock = new Mock(); + memberTypeServiceMock.Setup(x => x.GetDefault()).Returns("Member"); + var membershipServiceMock = new Mock(); + membershipServiceMock.Setup(service => service.Exists("test")).Returns(false); + membershipServiceMock.Setup(service => service.GetByEmail("test@test.com")).Returns(() => null); + membershipServiceMock.Setup( service => service.CreateWithIdentity(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Callback((string u, string e, string p, string m) => { @@ -154,20 +159,20 @@ namespace Umbraco.Tests.Membership }) .Returns(() => createdMember); - var provider = new MembersMembershipProvider(mServiceMock.Object); + var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object); provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Hashed" }, { "hashAlgorithmType", "HMACSHA256" } }); - + MembershipCreateStatus status; provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); Assert.AreNotEqual("test", createdMember.RawPasswordValue); - + string salt; var storedPassword = provider.StoredPassword(createdMember.RawPasswordValue, out salt); var hashedPassword = provider.EncryptOrHashPassword("testtest$1", salt); Assert.AreEqual(hashedPassword, storedPassword); } - + } } \ No newline at end of file diff --git a/src/Umbraco.Web.UI/config/ClientDependency.config b/src/Umbraco.Web.UI/config/ClientDependency.config index 3fb38db592..29c285ffc6 100644 --- a/src/Umbraco.Web.UI/config/ClientDependency.config +++ b/src/Umbraco.Web.UI/config/ClientDependency.config @@ -10,7 +10,7 @@ NOTES: * Compression/Combination/Minification is not enabled unless debug="false" is specified on the 'compiliation' element in the web.config * A new version will invalidate both client and server cache and create new persisted files --> - +