From 1a573fcb7553d1451a4fc402d6a78bfe666472d4 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Tue, 16 Mar 2021 10:44:55 +0100 Subject: [PATCH] Review fixes https://github.com/umbraco/Umbraco-CMS/pull/9968 --- .../Models/Identity/IIdentityUserToken.cs | 2 +- .../Services/IExternalLoginService.cs | 6 -- .../Migrations/Install/DatabaseBuilder.cs | 2 +- .../Security/UmbracoIdentityUser.cs | 5 +- .../Implement/ExternalLoginService.cs | 10 --- .../Services/ExternalLoginServiceTests.cs | 62 ++++--------------- src/Umbraco.Web.UI.NetCore/appsettings.json | 2 +- 7 files changed, 17 insertions(+), 72 deletions(-) diff --git a/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs b/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs index df0c97ff08..c3a451f31d 100644 --- a/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs +++ b/src/Umbraco.Core/Models/Identity/IIdentityUserToken.cs @@ -5,7 +5,7 @@ namespace Umbraco.Cms.Core.Models.Identity /// /// An external login provider token /// - public interface IIdentityUserToken : IEntity, IRememberBeingDirty + public interface IIdentityUserToken : IEntity { /// /// Gets or sets user Id for the user who owns this token diff --git a/src/Umbraco.Core/Services/IExternalLoginService.cs b/src/Umbraco.Core/Services/IExternalLoginService.cs index b0c53c8f86..8834a4b33f 100644 --- a/src/Umbraco.Core/Services/IExternalLoginService.cs +++ b/src/Umbraco.Core/Services/IExternalLoginService.cs @@ -55,12 +55,6 @@ namespace Umbraco.Cms.Core.Services /// void Save(int userId, IEnumerable tokens); - /// - /// Save a single external login record - /// - /// - void Save(IIdentityUserLogin login); // TODO: This is unused apart from tests, perhaps remove it? - /// /// Deletes all user logins - normally used when a member is deleted /// diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index e06818a589..0da54785f2 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -156,7 +156,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install { _configManipulator.SaveConnectionString(EmbeddedDatabaseConnectionString, Constants.DbProviderNames.SqlCe); - var path = _hostingEnvironment.MapPathContentRoot(Path.Combine("App_Data", "Umbraco.sdf")); + var path = _hostingEnvironment.MapPathContentRoot(Path.Combine(Constants.SystemDirectories.Data, "Umbraco.sdf")); if (File.Exists(path) == false) { // this should probably be in a "using (new SqlCeEngine)" clause but not sure diff --git a/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs b/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs index 34ba0d2a12..525e7f839a 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoIdentityUser.cs @@ -190,7 +190,7 @@ namespace Umbraco.Cms.Core.Models.Identity get { // return if it exists - if (_tokens != null) + if (_tokens is not null) { return _tokens; } @@ -198,7 +198,8 @@ namespace Umbraco.Cms.Core.Models.Identity _tokens = new ObservableCollection(); // if the callback is there and hasn't been created yet then execute it and populate the logins - if (_getTokens != null && !_getTokens.IsValueCreated) + // if (_getTokens != null && !_getTokens.IsValueCreated) + if (_getTokens?.IsValueCreated != true) { foreach (IIdentityUserToken l in _getTokens.Value) { diff --git a/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs b/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs index 03bd495fc2..79c827f641 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ExternalLoginService.cs @@ -72,16 +72,6 @@ namespace Umbraco.Cms.Core.Services.Implement } } - /// - public void Save(IIdentityUserLogin login) - { - using (var scope = ScopeProvider.CreateScope()) - { - _externalLoginRepository.Save(login); - scope.Complete(); - } - } - /// public void DeleteUserLogins(int userId) { diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs index b6a4e6bc4e..e56891601c 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ExternalLoginServiceTests.cs @@ -2,14 +2,13 @@ // See LICENSE for more details. using System; -using System.Collections.Generic; using System.Linq; using System.Threading; using NUnit.Framework; using Umbraco.Cms.Core.Models.Identity; -using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -22,13 +21,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services { private IUserService UserService => GetRequiredService(); - private IExternalLoginService ExternalLoginService => GetRequiredService(); + private IExternalLoginService ExternalLoginService => (IExternalLoginService)GetRequiredService(); [Test] [Ignore("We don't support duplicates anymore, this removing on save was a breaking change work around, this needs to be ported to a migration")] public void Removes_Existing_Duplicates_On_Save() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey = Guid.NewGuid().ToString("N"); @@ -76,7 +75,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Does_Not_Persist_Duplicates() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey = Guid.NewGuid().ToString("N"); @@ -92,49 +91,10 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services Assert.AreEqual(1, logins.Count); } - [Test] - public void Single_Create() - { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); - UserService.Save(user); - - var extLogin = new IdentityUserLogin("test1", Guid.NewGuid().ToString("N"), user.Id.ToString()) - { - UserData = "hello" - }; - ExternalLoginService.Save(extLogin); - - IEnumerable found = ExternalLoginService.GetExternalLogins(user.Id); - - Assert.AreEqual(1, found.Count()); - Assert.IsTrue(extLogin.HasIdentity); - Assert.IsTrue(extLogin.Id > 0); - } - - [Test] - public void Single_Update() - { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); - UserService.Save(user); - - var extLogin = new IdentityUserLogin("test1", Guid.NewGuid().ToString("N"), user.Id.ToString()) - { - UserData = "hello" - }; - ExternalLoginService.Save(extLogin); - - extLogin.UserData = "world"; - ExternalLoginService.Save(extLogin); - - var found = ExternalLoginService.GetExternalLogins(user.Id).ToList(); - Assert.AreEqual(1, found.Count); - Assert.AreEqual("world", found[0].UserData); - } - [Test] public void Multiple_Update() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey1 = Guid.NewGuid().ToString("N"); @@ -162,7 +122,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Can_Find_As_Extended_Type() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); string providerKey1 = Guid.NewGuid().ToString("N"); @@ -183,7 +143,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Add_Logins() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -206,7 +166,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Add_Tokens() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -231,7 +191,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Add_Update_Delete_Logins() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -265,7 +225,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Add_Update_Delete_Tokens() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] @@ -308,7 +268,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services [Test] public void Add_Retrieve_User_Data() { - var user = new User(GlobalSettings, "Test", "test@test.com", "test", "helloworldtest"); + var user = new UserBuilder().Build(); UserService.Save(user); ExternalLogin[] externalLogins = new[] diff --git a/src/Umbraco.Web.UI.NetCore/appsettings.json b/src/Umbraco.Web.UI.NetCore/appsettings.json index c7f192babb..0d0330b07b 100644 --- a/src/Umbraco.Web.UI.NetCore/appsettings.json +++ b/src/Umbraco.Web.UI.NetCore/appsettings.json @@ -38,7 +38,7 @@ "ConvertUrlsToAscii": "try" }, "RuntimeMinification": { - "dataFolder": "wmbraco/Data/TEMP/Smidge", + "dataFolder": "umbraco/Data/TEMP/Smidge", "version": "637510451273675926" }, "Security": {