From 7dee2802fefe3394330087fa205d54d9863fe43a Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Tue, 1 Oct 2013 10:20:16 +0200 Subject: [PATCH] Fixing Member and MemberType, so the ids are properly assigned through the repositories. Adding test to assert that no Properties, PropertyTypes or PropertyGroups exists without an id when loaded from the db. NOTE Changes signature of Member class as Email, Username and Password is required. --- src/Umbraco.Core/Models/Member.cs | 13 +- .../Rdbms/PropertyTypeGroupReadOnlyDto.cs | 3 + .../Factories/MemberReadOnlyFactory.cs | 9 +- .../Factories/MemberTypeReadOnlyFactory.cs | 5 +- .../Repositories/MemberTypeRepository.cs | 4 +- src/Umbraco.Core/Services/MemberService.cs | 10 +- .../Repositories/MemberRepositoryTest.cs | 116 ++++-------------- .../Repositories/MemberTypeRepositoryTest.cs | 50 +++++--- .../TestHelpers/Entities/MockedMember.cs | 3 +- .../Providers/MembersMembershipProvider.cs | 4 +- 10 files changed, 82 insertions(+), 135 deletions(-) diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index b3baac8351..335beb9ee4 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -20,19 +20,26 @@ namespace Umbraco.Core.Models private object _providerUserKey; private Type _userTypeKey; - public Member(string name, int parentId, IMemberType contentType, PropertyCollection properties) : base(name, parentId, contentType, properties) + public Member(string name, string email, string username, string password, int parentId, IMemberType contentType) + : base(name, parentId, contentType, new PropertyCollection()) { Mandate.ParameterNotNull(contentType, "contentType"); _contentType = contentType; + _email = email; + _username = username; + _password = password; } - public Member(string name, IContentBase parent, IMemberType contentType, PropertyCollection properties) - : base(name, parent, contentType, properties) + public Member(string name, string email, string username, string password, IContentBase parent, IMemberType contentType) + : base(name, parent, contentType, new PropertyCollection()) { Mandate.ParameterNotNull(contentType, "contentType"); _contentType = contentType; + _email = email; + _username = username; + _password = password; } private static readonly PropertyInfo DefaultContentTypeAliasSelector = ExpressionHelper.GetPropertyInfo(x => x.ContentTypeAlias); diff --git a/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs b/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs index 9f66813560..01736a270d 100644 --- a/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/PropertyTypeGroupReadOnlyDto.cs @@ -18,5 +18,8 @@ namespace Umbraco.Core.Models.Rdbms [Column("PropertyGroupSortOrder")] public int SortOrder { get; set; } + + [Column("contenttypeNodeId")] + public int ContentTypeNodeId { get; set; } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Factories/MemberReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberReadOnlyFactory.cs index 09d4765eb3..16879a7871 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberReadOnlyFactory.cs @@ -18,17 +18,13 @@ namespace Umbraco.Core.Persistence.Factories public IMember BuildEntity(MemberReadOnlyDto dto) { var properties = CreateProperties(_memberTypes[dto.ContentTypeAlias], dto.Properties, dto.CreateDate); - var propertyCollection = new PropertyCollection(properties); - var member = new Member(dto.Text, dto.ParentId, _memberTypes[dto.ContentTypeAlias], propertyCollection) + var member = new Member(dto.Text, dto.Email, dto.LoginName, dto.Password, dto.ParentId, _memberTypes[dto.ContentTypeAlias]) { Id = dto.NodeId, CreateDate = dto.CreateDate, UpdateDate = dto.UpdateDate, Name = dto.Text, - Email = dto.Email, - Username = dto.LoginName, - Password = dto.Password, ProviderUserKey = dto.UniqueId, Trashed = dto.Trashed, Key = dto.UniqueId.Value, @@ -37,7 +33,8 @@ namespace Umbraco.Core.Persistence.Factories Path = dto.Path, SortOrder = dto.SortOrder, Version = dto.VersionId, - ContentTypeAlias = dto.ContentTypeAlias + ContentTypeAlias = dto.ContentTypeAlias, + Properties = new PropertyCollection(properties) }; member.SetProviderUserKeyType(typeof(Guid)); diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index 925e0edccd..4884a18d98 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -60,7 +60,7 @@ namespace Umbraco.Core.Persistence.Factories { var group = new PropertyGroup(); //Only assign an Id if the PropertyGroup belongs to this ContentType - if (groupDto.Id.HasValue && groupDto.Id == memberType.Id) + if (groupDto.ContentTypeNodeId == memberType.Id) { group.Id = groupDto.Id.Value; @@ -78,7 +78,8 @@ namespace Umbraco.Core.Persistence.Factories group.PropertyTypes = new PropertyTypeCollection(); //Because we are likely to have a group with no PropertyTypes we need to ensure that these are excluded - var typeDtos = dto.PropertyTypes.Where(x => x.Id.HasValue && x.Id > 0 && x.PropertyTypeGroupId.HasValue && x.PropertyTypeGroupId.Value == groupDto.Id.Value); + var localGroupDto = groupDto; + var typeDtos = dto.PropertyTypes.Where(x => x.Id.HasValue && x.Id > 0 && x.PropertyTypeGroupId.HasValue && x.PropertyTypeGroupId.Value == localGroupDto.Id.Value); foreach (var typeDto in typeDtos) { //Internal dictionary for adding "MemberCanEdit" and "VisibleOnProfile" properties to each PropertyType diff --git a/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs index be3a9eb752..9ee215d4f7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MemberTypeRepository.cs @@ -103,8 +103,8 @@ namespace Umbraco.Core.Persistence.Repositories "cmsPropertyType.validationRegExp", "cmsPropertyType.dataTypeId", "cmsPropertyType.sortOrder AS PropertyTypeSortOrder", "cmsPropertyType.propertyTypeGroupId AS PropertyTypesGroupId", "cmsMemberType.memberCanEdit", "cmsMemberType.viewOnProfile", "cmsDataType.controlId", "cmsDataType.dbType", "cmsPropertyTypeGroup.id AS PropertyTypeGroupId", - "cmsPropertyTypeGroup.text AS PropertyGroupName", "cmsPropertyTypeGroup.parentGroupId", - "cmsPropertyTypeGroup.sortorder AS PropertyGroupSortOrder") + "cmsPropertyTypeGroup.text AS PropertyGroupName", "cmsPropertyTypeGroup.parentGroupId", + "cmsPropertyTypeGroup.sortorder AS PropertyGroupSortOrder", "cmsPropertyTypeGroup.contenttypeNodeId") .From() .InnerJoin().On(left => left.NodeId, right => right.NodeId) .LeftJoin().On(left => left.ContentTypeId, right => right.NodeId) diff --git a/src/Umbraco.Core/Services/MemberService.cs b/src/Umbraco.Core/Services/MemberService.cs index 237e7286c0..914ef70e60 100644 --- a/src/Umbraco.Core/Services/MemberService.cs +++ b/src/Umbraco.Core/Services/MemberService.cs @@ -200,13 +200,13 @@ namespace Umbraco.Core.Services /// /// Creates a new Member /// - /// /// + /// /// /// /// /// - public IMember CreateMember(string username, string email, string password, string memberTypeAlias, int userId = 0) + public IMember CreateMember(string email, string username, string password, string memberTypeAlias, int userId = 0) { var uow = _uowProvider.GetUnitOfWork(); IMemberType memberType; @@ -220,14 +220,10 @@ namespace Umbraco.Core.Services if (memberType == null) throw new Exception(string.Format("No MemberType matching the passed in Alias: '{0}' was found", memberTypeAlias)); - var member = new Member(email, -1, memberType, new PropertyCollection()); + var member = new Member(email, email, username, password, -1, memberType); using (var repository = _repositoryFactory.CreateMemberRepository(uow)) { - member.Username = username; - member.Email = email; - member.Password = password; - repository.AddOrUpdate(member); uow.Commit(); } diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs index 6795f2c608..451110e0b1 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberRepositoryTest.cs @@ -4,32 +4,23 @@ using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; -using Umbraco.Core.ObjectResolution; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Caching; -using Umbraco.Core.Persistence.Mappers; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; -using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; -using Umbraco.Core.Publishing; -using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; namespace Umbraco.Tests.Persistence.Repositories { - [TestFixture, NUnit.Framework.Ignore] - public class MemberRepositoryTest : MemberRepositoryBaseTest + [TestFixture] + public class MemberRepositoryTest : BaseDatabaseFactoryTest { - #region Overrides of MemberRepositoryBaseTest - [SetUp] public override void Initialize() { base.Initialize(); - - SqlSyntaxContext.SqlSyntaxProvider = SqlServerSyntax.Provider; } [TearDown] @@ -38,18 +29,6 @@ namespace Umbraco.Tests.Persistence.Repositories base.TearDown(); } - public override string ConnectionString - { - get { return @"server=.\SQLEXPRESS;database=EmptyForTest;user id=umbraco;password=umbraco"; } - } - - public override string ProviderName - { - get { return "System.Data.SqlClient"; } - } - - #endregion - private MemberRepository CreateRepository(IDatabaseUnitOfWork unitOfWork, out MemberTypeRepository memberTypeRepository) { memberTypeRepository = new MemberTypeRepository(unitOfWork, NullCacheProvider.Current); @@ -71,10 +50,11 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(repository, Is.Not.Null); } - [Test] + [Test, NUnit.Framework.Ignore] public void MemberRepository_Can_Get_Member_By_Id() { - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -85,10 +65,11 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] + [Test, NUnit.Framework.Ignore] public void MemberRepository_Can_Get_Specific_Members() { - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -101,10 +82,11 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] + [Test, NUnit.Framework.Ignore] public void MemberRepository_Can_Get_All_Members() { - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -117,11 +99,12 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] + [Test, NUnit.Framework.Ignore] public void MemberRepository_Can_Perform_GetByQuery_With_Key() { // Arrange - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -136,11 +119,12 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] + [Test, NUnit.Framework.Ignore] public void MemberRepository_Can_Perform_GetByQuery_With_Property_Value() { // Arrange - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -156,11 +140,12 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] + [Test, NUnit.Framework.Ignore] public void MemberRepository_Can_Perform_GetByQuery_With_Property_Alias_And_Value() { // Arrange - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -179,7 +164,8 @@ namespace Umbraco.Tests.Persistence.Repositories public void MemberRepository_Can_Persist_Member() { IMember sut; - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); MemberTypeRepository memberTypeRepository; using (var repository = CreateRepository(unitOfWork, out memberTypeRepository)) { @@ -196,7 +182,9 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(sut, Is.Not.Null); Assert.That(sut.ContentType.PropertyGroups.Count(), Is.EqualTo(1)); Assert.That(sut.ContentType.PropertyTypes.Count(), Is.EqualTo(12)); + Assert.That(sut.Properties.Count(), Is.EqualTo(12)); + Assert.That(sut.Properties.Any(x => x.HasIdentity == false || x.Id == 0), Is.False); } } @@ -278,60 +266,4 @@ namespace Umbraco.Tests.Persistence.Repositories get { return new Guid(Constants.ObjectTypes.Member); } } } - - [TestFixture] - public abstract class MemberRepositoryBaseTest - { - [SetUp] - public virtual void Initialize() - { - TestHelper.SetupLog4NetForTests(); - TestHelper.InitializeContentDirectories(); - - string path = TestHelper.CurrentAssemblyDirectory; - AppDomain.CurrentDomain.SetData("DataDirectory", path); - - RepositoryResolver.Current = new RepositoryResolver( - new RepositoryFactory()); - - MappingResolver.Current = new MappingResolver( - () => PluginManager.Current.ResolveAssignedMapperTypes()); - - - var dbFactory = new DefaultDatabaseFactory(ConnectionString, ProviderName); - UnitOfWorkProvider = new PetaPocoUnitOfWorkProvider(dbFactory); - - ApplicationContext.Current = new ApplicationContext( - //assign the db context - new DatabaseContext(new DefaultDatabaseFactory()), - //assign the service context - new ServiceContext(UnitOfWorkProvider, new FileUnitOfWorkProvider(), new PublishingStrategy()), - //disable cache - false) - { - IsReady = true - }; - - Resolution.Freeze(); - } - - [TearDown] - public virtual void TearDown() - { - SqlSyntaxContext.SqlSyntaxProvider = null; - AppDomain.CurrentDomain.SetData("DataDirectory", null); - - //reset the app context - ApplicationContext.Current = null; - - RepositoryResolver.Reset(); - MappingResolver.Reset(); - } - - public abstract string ConnectionString { get; } - - public abstract string ProviderName { get; } - - public PetaPocoUnitOfWorkProvider UnitOfWorkProvider { get; set; } - } } \ No newline at end of file diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs index c9a88878d4..e7a7285f7d 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -1,24 +1,22 @@ using System.Linq; using NUnit.Framework; +using Umbraco.Core.Models; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Caching; using Umbraco.Core.Persistence.Repositories; -using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.TestHelpers.Entities; namespace Umbraco.Tests.Persistence.Repositories { - [TestFixture, NUnit.Framework.Ignore] - public class MemberTypeRepositoryTest : MemberRepositoryBaseTest + [TestFixture] + public class MemberTypeRepositoryTest : BaseDatabaseFactoryTest { - #region Overrides of MemberRepositoryBaseTest - [SetUp] public override void Initialize() { base.Initialize(); - - SqlSyntaxContext.SqlSyntaxProvider = SqlServerSyntax.Provider; } [TearDown] @@ -27,18 +25,6 @@ namespace Umbraco.Tests.Persistence.Repositories base.TearDown(); } - public override string ConnectionString - { - get { return @"server=.\SQLEXPRESS;database=EmptyForTest;user id=umbraco;password=umbraco"; } - } - - public override string ProviderName - { - get { return "System.Data.SqlClient"; } - } - - #endregion - private MemberTypeRepository CreateRepository(IDatabaseUnitOfWork unitOfWork) { return new MemberTypeRepository(unitOfWork, NullCacheProvider.Current); @@ -59,9 +45,33 @@ namespace Umbraco.Tests.Persistence.Repositories } [Test] + public void MemberRepository_Can_Persist_Member() + { + IMemberType sut; + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + using (var repository = CreateRepository(unitOfWork)) + { + var memberType = MockedContentTypes.CreateSimpleMemberType(); + repository.AddOrUpdate(memberType); + unitOfWork.Commit(); + + sut = repository.Get(memberType.Id); + + Assert.That(sut, Is.Not.Null); + Assert.That(sut.PropertyGroups.Count(), Is.EqualTo(1)); + Assert.That(sut.PropertyTypes.Count(), Is.EqualTo(12)); + + Assert.That(sut.PropertyGroups.Any(x => x.HasIdentity == false || x.Id == 0), Is.False); + Assert.That(sut.PropertyTypes.Any(x => x.HasIdentity == false || x.Id == 0), Is.False); + } + } + + [Test, NUnit.Framework.Ignore] public void MemberTypeRepository_Can_Get_MemberType_By_Id() { - var unitOfWork = UnitOfWorkProvider.GetUnitOfWork(); + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); using (var repository = CreateRepository(unitOfWork)) { diff --git a/src/Umbraco.Tests/TestHelpers/Entities/MockedMember.cs b/src/Umbraco.Tests/TestHelpers/Entities/MockedMember.cs index 4aeb406811..2e019b5ae5 100644 --- a/src/Umbraco.Tests/TestHelpers/Entities/MockedMember.cs +++ b/src/Umbraco.Tests/TestHelpers/Entities/MockedMember.cs @@ -1,4 +1,5 @@ using Umbraco.Core.Models; +using Umbraco.Core.Models.Membership; namespace Umbraco.Tests.TestHelpers.Entities { @@ -6,7 +7,7 @@ namespace Umbraco.Tests.TestHelpers.Entities { public static Member CreateSimpleContent(IMemberType contentType, string name, string email, string password, string username, int parentId) { - var member = new Member(name, parentId, contentType, new PropertyCollection()) + var member = new Member(name, email, username, password, parentId, contentType) { CreatorId = 0, Email = email, diff --git a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs index 0f7d255f74..b02ed4ecf2 100644 --- a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs @@ -323,8 +323,8 @@ namespace Umbraco.Web.Security.Providers "Cannot create member as a member with the same email address exists: " + email); return null; } - - var member = MemberService.CreateMember(username, email, password, DefaultMemberTypeAlias); + + var member = MemberService.CreateMember(email, username, password, DefaultMemberTypeAlias); member.IsApproved = isApproved; member.PasswordQuestion = passwordQuestion;