From 9a1de6468b369588fb2fbf9473f2d7d81cc1e785 Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Wed, 19 Aug 2020 13:33:49 +1000
Subject: [PATCH] notes and unit test, just wanted to see if this query could
 be improved but it can't

---
 .../Repositories/Implement/UserRepository.cs  |  8 +--
 .../Repositories/UserRepositoryTest.cs        | 61 ++++++++++++++-----
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs
index 3be5102b83..4721037674 100644
--- a/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs
+++ b/src/Umbraco.Core/Persistence/Repositories/Implement/UserRepository.cs
@@ -168,10 +168,7 @@ ORDER BY colName";
         }
 
         public Guid CreateLoginSession(int userId, string requestingIpAddress, bool cleanStaleSessions = true)
-        {
-            // TODO: I know this doesn't follow the normal repository conventions which would require us to create a UserSessionRepository
-            //and also business logic models for these objects but that's just so overkill for what we are doing
-            //and now that everything is properly in a transaction (Scope) there doesn't seem to be much reason for using that anymore
+        {            
             var now = DateTime.UtcNow;
             var dto = new UserLoginDto
             {
@@ -201,13 +198,14 @@ ORDER BY colName";
             // that query is going to run a *lot*, make it a template
             var t = SqlContext.Templates.Get("Umbraco.Core.UserRepository.ValidateLoginSession", s => s
                 .Select<UserLoginDto>()
+                .SelectTop(1)
                 .From<UserLoginDto>()
                 .Where<UserLoginDto>(x => x.SessionId == SqlTemplate.Arg<Guid>("sessionId"))
                 .ForUpdate());
 
             var sql = t.Sql(sessionId);
 
-            var found = Database.Query<UserLoginDto>(sql).FirstOrDefault();
+            var found = Database.FirstOrDefault<UserLoginDto>(sql);
             if (found == null || found.UserId != userId || found.LoggedOutUtc.HasValue)
                 return false;
 
diff --git a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs
index bbefb79f6b..b2efbd34b8 100644
--- a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs
+++ b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs
@@ -16,6 +16,7 @@ using Umbraco.Tests.Testing;
 using Umbraco.Core.Persistence;
 using Umbraco.Core.PropertyEditors;
 using System;
+using Umbraco.Core.Persistence.Dtos;
 
 namespace Umbraco.Tests.Persistence.Repositories
 {
@@ -76,12 +77,44 @@ namespace Umbraco.Tests.Persistence.Repositories
             return new UserGroupRepository(accessor, AppCaches.Disabled, Logger);
         }
 
+        [Test]
+        public void Validate_Login_Session()
+        {
+            // Arrange
+            var provider = TestObjects.GetScopeProvider(Logger);
+            var user = MockedUser.CreateUser();
+            using (var scope = provider.CreateScope(autoComplete: true))
+            {
+                var repository = CreateRepository(provider);                
+                repository.Save(user);                
+            }
+
+            using (var scope = provider.CreateScope(autoComplete: true))
+            {
+                var repository = CreateRepository(provider);
+                var sessionId = repository.CreateLoginSession(user.Id, "1.2.3.4");
+
+                // manually update this record to be in the past
+                scope.Database.Execute(SqlContext.Sql()
+                    .Update<UserLoginDto>(u => u.Set(x => x.LoggedOutUtc, DateTime.UtcNow.AddDays(-100)))
+                    .Where<UserLoginDto>(x => x.SessionId == sessionId));
+
+                var isValid = repository.ValidateLoginSession(user.Id, sessionId);
+                Assert.IsFalse(isValid);
+
+                // create a new one
+                sessionId = repository.CreateLoginSession(user.Id, "1.2.3.4");
+                isValid = repository.ValidateLoginSession(user.Id, sessionId);
+                Assert.IsTrue(isValid);
+            }
+        }
+
         [Test]
         public void Can_Perform_Add_On_UserRepository()
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -101,7 +134,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -125,7 +158,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -150,7 +183,7 @@ namespace Umbraco.Tests.Persistence.Repositories
 
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var userRepository = CreateRepository(provider);
                 var contentRepository = CreateContentRepository(provider, out var contentTypeRepo);
@@ -209,7 +242,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -237,7 +270,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
                 var userGroupRepository = CreateUserGroupRepository(provider);
@@ -260,7 +293,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -280,7 +313,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -301,7 +334,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -322,7 +355,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -341,7 +374,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -360,7 +393,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         public void Can_Get_Paged_Results_By_Query_And_Filter_And_Groups()
         {
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -393,7 +426,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         public void Can_Get_Paged_Results_With_Filter_And_Groups()
         {
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
 
@@ -426,7 +459,7 @@ namespace Umbraco.Tests.Persistence.Repositories
         {
             // Arrange
             var provider = TestObjects.GetScopeProvider(Logger);
-            using (var scope = provider.CreateScope())
+            using (var scope = provider.CreateScope(autoComplete: true))
             {
                 var repository = CreateRepository(provider);
                 var userGroupRepository = CreateUserGroupRepository(provider);