From 488f6925b889ec0841669a24002cf54901b7614d Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 26 Nov 2019 12:49:57 +1100 Subject: [PATCH] Fixes tests, removes some magic strings --- .../PublishedContent/PublishedContentType.cs | 19 +++++---- src/Umbraco.Core/Models/Member.cs | 28 ++++++------- .../Repositories/Implement/UserRepository.cs | 1 + src/Umbraco.Tests/App.config | 1 - .../UmbracoServiceMembershipProviderTests.cs | 36 +---------------- .../Services/MemberServiceTests.cs | 39 +++++++++---------- .../Testing/TestingTests/MockTests.cs | 11 ++++-- .../PublishedCache/NuCache/PublishedMember.cs | 18 ++++----- .../PublishedCache/PublishedMember.cs | 18 ++++----- 9 files changed, 68 insertions(+), 103 deletions(-) diff --git a/src/Umbraco.Abstractions/Models/PublishedContent/PublishedContentType.cs b/src/Umbraco.Abstractions/Models/PublishedContent/PublishedContentType.cs index b11e991118..cf2afb8706 100644 --- a/src/Umbraco.Abstractions/Models/PublishedContent/PublishedContentType.cs +++ b/src/Umbraco.Abstractions/Models/PublishedContent/PublishedContentType.cs @@ -102,16 +102,15 @@ namespace Umbraco.Core.Models.PublishedContent // TODO: this list somehow also exists in constants, see memberTypeRepository => remove duplicate! private static readonly Dictionary BuiltinMemberProperties = new Dictionary { - { "Email", Constants.DataTypes.Textbox }, - { "Username", Constants.DataTypes.Textbox }, - { "PasswordQuestion", Constants.DataTypes.Textbox }, - { "Comments", Constants.DataTypes.Textbox }, - { "IsApproved", Constants.DataTypes.Boolean }, - { "IsLockedOut", Constants.DataTypes.Boolean }, - { "LastLockoutDate", Constants.DataTypes.DateTime }, - { "CreateDate", Constants.DataTypes.DateTime }, - { "LastLoginDate", Constants.DataTypes.DateTime }, - { "LastPasswordChangeDate", Constants.DataTypes.DateTime }, + { nameof(IMember.Email), Constants.DataTypes.Textbox }, + { nameof(IMember.Username), Constants.DataTypes.Textbox }, + { nameof(IMember.Comments), Constants.DataTypes.Textbox }, + { nameof(IMember.IsApproved), Constants.DataTypes.Boolean }, + { nameof(IMember.IsLockedOut), Constants.DataTypes.Boolean }, + { nameof(IMember.LastLockoutDate), Constants.DataTypes.DateTime }, + { nameof(IMember.CreateDate), Constants.DataTypes.DateTime }, + { nameof(IMember.LastLoginDate), Constants.DataTypes.DateTime }, + { nameof(IMember.LastPasswordChangeDate), Constants.DataTypes.DateTime }, }; #region Content type diff --git a/src/Umbraco.Core/Models/Member.cs b/src/Umbraco.Core/Models/Member.cs index 3d4940c348..39724245ca 100644 --- a/src/Umbraco.Core/Models/Member.cs +++ b/src/Umbraco.Core/Models/Member.cs @@ -177,7 +177,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.Comments, "Comments", default(string)); + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.Comments, nameof(Comments), default(string)); if (a.Success == false) return a.Result; return Properties[Constants.Conventions.Member.Comments].GetValue() == null @@ -188,7 +188,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.Comments, - "Comments") == false) return; + nameof(Comments)) == false) return; Properties[Constants.Conventions.Member.Comments].SetValue(value); } @@ -206,7 +206,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.IsApproved, "IsApproved", + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.IsApproved, nameof(IsApproved), //This is the default value if the prop is not found true); if (a.Success == false) return a.Result; @@ -223,7 +223,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.IsApproved, - "IsApproved") == false) return; + nameof(IsApproved)) == false) return; Properties[Constants.Conventions.Member.IsApproved].SetValue(value); } @@ -241,7 +241,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.IsLockedOut, "IsLockedOut", false); + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.IsLockedOut, nameof(IsLockedOut), false); if (a.Success == false) return a.Result; if (Properties[Constants.Conventions.Member.IsLockedOut].GetValue() == null) return false; var tryConvert = Properties[Constants.Conventions.Member.IsLockedOut].GetValue().TryConvertTo(); @@ -256,7 +256,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.IsLockedOut, - "IsLockedOut") == false) return; + nameof(IsLockedOut)) == false) return; Properties[Constants.Conventions.Member.IsLockedOut].SetValue(value); } @@ -274,7 +274,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastLoginDate, "LastLoginDate", default(DateTime)); + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastLoginDate, nameof(LastLoginDate), default(DateTime)); if (a.Success == false) return a.Result; if (Properties[Constants.Conventions.Member.LastLoginDate].GetValue() == null) return default(DateTime); var tryConvert = Properties[Constants.Conventions.Member.LastLoginDate].GetValue().TryConvertTo(); @@ -289,7 +289,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.LastLoginDate, - "LastLoginDate") == false) return; + nameof(LastLoginDate)) == false) return; Properties[Constants.Conventions.Member.LastLoginDate].SetValue(value); } @@ -307,7 +307,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastPasswordChangeDate, "LastPasswordChangeDate", default(DateTime)); + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastPasswordChangeDate, nameof(LastPasswordChangeDate), default(DateTime)); if (a.Success == false) return a.Result; if (Properties[Constants.Conventions.Member.LastPasswordChangeDate].GetValue() == null) return default(DateTime); var tryConvert = Properties[Constants.Conventions.Member.LastPasswordChangeDate].GetValue().TryConvertTo(); @@ -322,7 +322,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.LastPasswordChangeDate, - "LastPasswordChangeDate") == false) return; + nameof(LastPasswordChangeDate)) == false) return; Properties[Constants.Conventions.Member.LastPasswordChangeDate].SetValue(value); } @@ -340,7 +340,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastLockoutDate, "LastLockoutDate", default(DateTime)); + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.LastLockoutDate, nameof(LastLockoutDate), default(DateTime)); if (a.Success == false) return a.Result; if (Properties[Constants.Conventions.Member.LastLockoutDate].GetValue() == null) return default(DateTime); var tryConvert = Properties[Constants.Conventions.Member.LastLockoutDate].GetValue().TryConvertTo(); @@ -355,7 +355,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.LastLockoutDate, - "LastLockoutDate") == false) return; + nameof(LastLockoutDate)) == false) return; Properties[Constants.Conventions.Member.LastLockoutDate].SetValue(value); } @@ -374,7 +374,7 @@ namespace Umbraco.Core.Models { get { - var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.FailedPasswordAttempts, "FailedPasswordAttempts", 0); + var a = WarnIfPropertyTypeNotFoundOnGet(Constants.Conventions.Member.FailedPasswordAttempts, nameof(FailedPasswordAttempts), 0); if (a.Success == false) return a.Result; if (Properties[Constants.Conventions.Member.FailedPasswordAttempts].GetValue() == null) return default(int); var tryConvert = Properties[Constants.Conventions.Member.FailedPasswordAttempts].GetValue().TryConvertTo(); @@ -389,7 +389,7 @@ namespace Umbraco.Core.Models { if (WarnIfPropertyTypeNotFoundOnSet( Constants.Conventions.Member.FailedPasswordAttempts, - "FailedPasswordAttempts") == false) return; + nameof(FailedPasswordAttempts)) == false) return; Properties[Constants.Conventions.Member.FailedPasswordAttempts].SetValue(value); } diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs index ff8beecff5..f8e828a635 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs @@ -492,6 +492,7 @@ ORDER BY colName"; // list the columns to save, NOTE: would be nice to not have hard coded strings here but no real good way around that var colsToSave = new Dictionary { + //TODO: Change these to constants + nameof {"userDisabled", "IsApproved"}, {"userNoConsole", "IsLockedOut"}, {"startStructureID", "StartContentId"}, diff --git a/src/Umbraco.Tests/App.config b/src/Umbraco.Tests/App.config index 9eeb93384d..812e4383de 100644 --- a/src/Umbraco.Tests/App.config +++ b/src/Umbraco.Tests/App.config @@ -43,7 +43,6 @@ - diff --git a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs index 5a33cfb731..c40c91445e 100644 --- a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs +++ b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs @@ -73,41 +73,7 @@ namespace Umbraco.Tests.Membership Assert.IsNull(user); } - - [Test] - public void Password_Encrypted() - { - IMember createdMember = null; - var memberType = MockedContentTypes.CreateSimpleMemberType(); - foreach (var p in ConventionsHelper.GetStandardPropertyTypeStubs()) - { - memberType.AddPropertyType(p.Value); - } - 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(), It.IsAny())) - .Callback((string u, string e, string p, string m, bool isApproved) => - { - createdMember = new Member("test", e, u, p, memberType, isApproved); - }) - .Returns(() => createdMember); - - var provider = new MembersMembershipProvider(membershipServiceMock.Object, memberTypeServiceMock.Object, TestHelper.GetUmbracoVersion()); - provider.Initialize("test", new NameValueCollection { { "passwordFormat", "Encrypted" } }); - - - MembershipCreateStatus status; - provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); - - Assert.AreNotEqual("test", createdMember.RawPasswordValue); - var decrypted = provider.DecryptPassword(createdMember.RawPasswordValue); - Assert.AreEqual("testtest$1", decrypted); - } - + [Test] public void Password_Hashed_With_Salt() { diff --git a/src/Umbraco.Tests/Services/MemberServiceTests.cs b/src/Umbraco.Tests/Services/MemberServiceTests.cs index a0799c6856..e567fb4e5e 100644 --- a/src/Umbraco.Tests/Services/MemberServiceTests.cs +++ b/src/Umbraco.Tests/Services/MemberServiceTests.cs @@ -65,33 +65,30 @@ namespace Umbraco.Tests.Services // TODO: see TODO in PublishedContentType, this list contains duplicates var aliases = new[] - { - "umbracoMemberPasswordRetrievalQuestion", - "umbracoMemberPasswordRetrievalAnswer", - "umbracoMemberComments", - "umbracoMemberFailedPasswordAttempts", - "umbracoMemberApproved", - "umbracoMemberLockedOut", - "umbracoMemberLastLockoutDate", - "umbracoMemberLastLogin", - "umbracoMemberLastPasswordChangeDate", - "Email", - "Username", - "PasswordQuestion", - "Comments", - "IsApproved", - "IsLockedOut", - "LastLockoutDate", - "CreateDate", - "LastLoginDate", - "LastPasswordChangeDate" + { + Constants.Conventions.Member.Comments, + Constants.Conventions.Member.FailedPasswordAttempts, + Constants.Conventions.Member.IsApproved, + Constants.Conventions.Member.IsLockedOut, + Constants.Conventions.Member.LastLockoutDate, + Constants.Conventions.Member.LastLoginDate, + Constants.Conventions.Member.LastPasswordChangeDate, + nameof(IMember.Email), + nameof(IMember.Username), + nameof(IMember.Comments), + nameof(IMember.IsApproved), + nameof(IMember.IsLockedOut), + nameof(IMember.LastLockoutDate), + nameof(IMember.CreateDate), + nameof(IMember.LastLoginDate), + nameof(IMember.LastPasswordChangeDate) }; var properties = pmember.Properties.ToList(); Assert.IsTrue(properties.Select(x => x.Alias).ContainsAll(aliases)); - var email = properties[aliases.IndexOf("Email")]; + var email = properties[aliases.IndexOf(nameof(IMember.Email))]; Assert.AreEqual("xemail", email.GetSourceValue()); } diff --git a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs index 1c007d1e49..1bcbc1e420 100644 --- a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs +++ b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs @@ -99,12 +99,15 @@ namespace Umbraco.Tests.Testing.TestingTests { var umbracoContext = TestObjects.GetUmbracoContextMock(); - var membershipHelper = new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), AppCaches.Disabled, Mock.Of()); + var logger = Mock.Of(); + var memberService = Mock.Of(); + var memberTypeService = Mock.Of(); + var membershipProvider = new MembersMembershipProvider(memberService, memberTypeService, Mock.Of()); + var membershipHelper = new MembershipHelper(umbracoContext.HttpContext, Mock.Of(), membershipProvider, Mock.Of(), memberService, memberTypeService, Mock.Of(), Mock.Of(), AppCaches.Disabled, logger); var umbracoHelper = new UmbracoHelper(Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), membershipHelper); var umbracoMapper = new UmbracoMapper(new MapDefinitionCollection(new[] { Mock.Of() })); - - // ReSharper disable once UnusedVariable - var umbracoApiController = new FakeUmbracoApiController(Mock.Of(), Mock.Of(), Mock.Of(), ServiceContext.CreatePartial(), AppCaches.NoCache, Mock.Of(), Mock.Of(), umbracoHelper, umbracoMapper); + + var umbracoApiController = new FakeUmbracoApiController(Mock.Of(), Mock.Of(), Mock.Of(), ServiceContext.CreatePartial(), AppCaches.NoCache, logger, Mock.Of(), umbracoHelper, umbracoMapper); Assert.Pass(); } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedMember.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedMember.cs index 9e47802c08..118831d305 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedMember.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedMember.cs @@ -74,15 +74,15 @@ namespace Umbraco.Web.PublishedCache.NuCache .ToDictionary(x => x.Alias, x => new[] { new PropertyData { Value = x.GetValue(), Culture = string.Empty, Segment = string.Empty } }, StringComparer.OrdinalIgnoreCase); // see also PublishedContentType - AddIf(contentType, properties, "Email", member.Email); - AddIf(contentType, properties, "Username", member.Username); - AddIf(contentType, properties, "Comments", member.Comments); - AddIf(contentType, properties, "IsApproved", member.IsApproved); - AddIf(contentType, properties, "IsLockedOut", member.IsLockedOut); - AddIf(contentType, properties, "LastLockoutDate", member.LastLockoutDate); - AddIf(contentType, properties, "CreateDate", member.CreateDate); - AddIf(contentType, properties, "LastLoginDate", member.LastLoginDate); - AddIf(contentType, properties, "LastPasswordChangeDate", member.LastPasswordChangeDate); + AddIf(contentType, properties, nameof(IMember.Email), member.Email); + AddIf(contentType, properties, nameof(IMember.Username), member.Username); + AddIf(contentType, properties, nameof(IMember.Comments), member.Comments); + AddIf(contentType, properties, nameof(IMember.IsApproved), member.IsApproved); + AddIf(contentType, properties, nameof(IMember.IsLockedOut), member.IsLockedOut); + AddIf(contentType, properties, nameof(IMember.LastLockoutDate), member.LastLockoutDate); + AddIf(contentType, properties, nameof(IMember.CreateDate), member.CreateDate); + AddIf(contentType, properties, nameof(IMember.LastLoginDate), member.LastLoginDate); + AddIf(contentType, properties, nameof(IMember.LastPasswordChangeDate), member.LastPasswordChangeDate); return properties; } diff --git a/src/Umbraco.Web/PublishedCache/PublishedMember.cs b/src/Umbraco.Web/PublishedCache/PublishedMember.cs index b26e9b90a5..2c3ba1d7ea 100644 --- a/src/Umbraco.Web/PublishedCache/PublishedMember.cs +++ b/src/Umbraco.Web/PublishedCache/PublishedMember.cs @@ -94,15 +94,15 @@ namespace Umbraco.Web.PublishedCache { var aliases = properties.Select(x => x.Alias).ToList(); - EnsureMemberProperty(properties, aliases, "Email", Email); - EnsureMemberProperty(properties, aliases, "UserName", UserName); - EnsureMemberProperty(properties, aliases, "Comments", Comments); - EnsureMemberProperty(properties, aliases, "IsApproved", IsApproved); - EnsureMemberProperty(properties, aliases, "IsLockedOut", IsLockedOut); - EnsureMemberProperty(properties, aliases, "LastLockoutDate", LastLockoutDate); - EnsureMemberProperty(properties, aliases, "CreateDate", CreateDate); - EnsureMemberProperty(properties, aliases, "LastLoginDate", LastLoginDate); - EnsureMemberProperty(properties, aliases, "LastPasswordChangeDate", LastPasswordChangeDate); + EnsureMemberProperty(properties, aliases, nameof(IMember.Email), Email); + EnsureMemberProperty(properties, aliases, nameof(IMember.Username), UserName); + EnsureMemberProperty(properties, aliases, nameof(IMember.Comments), Comments); + EnsureMemberProperty(properties, aliases, nameof(IMember.IsApproved), IsApproved); + EnsureMemberProperty(properties, aliases, nameof(IMember.IsLockedOut), IsLockedOut); + EnsureMemberProperty(properties, aliases, nameof(IMember.LastLockoutDate), LastLockoutDate); + EnsureMemberProperty(properties, aliases, nameof(IMember.CreateDate), CreateDate); + EnsureMemberProperty(properties, aliases, nameof(IMember.LastLoginDate), LastLoginDate); + EnsureMemberProperty(properties, aliases, nameof(IMember.LastPasswordChangeDate), LastPasswordChangeDate); } private void EnsureMemberProperty(List properties, List aliases, string alias, object value)