Merge remote-tracking branch 'origin/netcore/netcore' into netcore/feature/migrate-logging

Signed-off-by: Bjarke Berg <mail@bergmania.dk>

# Conflicts:
#	src/Umbraco.Infrastructure/Search/ExamineComponent.cs
#	src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs

src/Umbraco.Infrastructure/Persistence/Repositories/IMemberRepository.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using Umbraco.Core.Models;
 using Umbraco.Core.Persistence.Querying;
 
@@ -6,6 +7,8 @@ namespace Umbraco.Core.Persistence.Repositories
 {
     public interface IMemberRepository : IContentRepository<int, IMember>
     {
+        IMember GetByUsername(string username);
+
         /// <summary>
         /// Finds members in a given role
         /// </summary>
@@ -35,5 +38,17 @@ namespace Umbraco.Core.Persistence.Repositories
         /// <param name="query"></param>
         /// <returns></returns>
         int GetCountByQuery(IQuery<IMember> query);
+
+        /// <summary>
+        /// Sets a members last login date based on their username
+        /// </summary>
+        /// <param name="username"></param>
+        /// <param name="date"></param>
+        /// <remarks>
+        /// This is a specialized method because whenever a member logs in, the membership provider requires us to set the 'online' which requires
+        /// updating their login date. This operation must be fast and cannot use database locks which is fine if we are only executing a single query
+        /// for this data since there won't be any other data contention issues.
+        /// </remarks>
+        void SetLastLogin(string username, DateTime date);
     }
 }

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs

@@ -26,6 +26,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
         private readonly ITagRepository _tagRepository;
         private readonly IPasswordHasher _passwordHasher;
         private readonly IMemberGroupRepository _memberGroupRepository;
+        private readonly IRepositoryCachePolicy<IMember, string> _memberByUsernameCachePolicy;
 
         public MemberRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogger<MemberRepository> logger,
             IMemberTypeRepository memberTypeRepository, IMemberGroupRepository memberGroupRepository, ITagRepository tagRepository, ILanguageRepository languageRepository, IRelationRepository relationRepository, IRelationTypeRepository relationTypeRepository,
@@ -39,6 +40,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             _tagRepository = tagRepository ?? throw new ArgumentNullException(nameof(tagRepository));
             _passwordHasher = passwordHasher;
             _memberGroupRepository = memberGroupRepository;
+
+            _memberByUsernameCachePolicy = new DefaultRepositoryCachePolicy<IMember, string>(GlobalIsolatedCache, ScopeAccessor, DefaultOptions);
         }
 
         protected override MemberRepository This => this;
@@ -383,12 +386,27 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             if (changedCols.Count > 0)
                 Database.Update(dto, changedCols);
 
-            // replace the property data
-            var deletePropertyDataSql = SqlContext.Sql().Delete<PropertyDataDto>().Where<PropertyDataDto>(x => x.VersionId == member.VersionId);
-            Database.Execute(deletePropertyDataSql);
+            // Replace the property data
+            // Lookup the data to update with a UPDLOCK (using ForUpdate()) this is because we have another method that doesn't take an explicit WriteLock
+            // in SetLastLogin which is called very often and we want to avoid the lock timeout for the explicit lock table but we still need to ensure atomic
+            // operations between that method and this one. 
+
+            var propDataSql = SqlContext.Sql().Select("*").From<PropertyDataDto>().Where<PropertyDataDto>(x => x.VersionId == member.VersionId).ForUpdate();
+            var existingPropData = Database.Fetch<PropertyDataDto>(propDataSql).ToDictionary(x => x.PropertyTypeId);
             var propertyDataDtos = PropertyFactory.BuildDtos(member.ContentType.Variations, member.VersionId, 0, entity.Properties, LanguageRepository, out _, out _);
             foreach (var propertyDataDto in propertyDataDtos)
-                Database.Insert(propertyDataDto);
+            {
+                // Check if this already exists and update, else insert a new one
+                if (existingPropData.TryGetValue(propertyDataDto.PropertyTypeId, out var propData))
+                {
+                    propertyDataDto.Id = propData.Id;
+                    Database.Update(propertyDataDto);
+                }
+                else
+                {
+                    Database.Insert(propertyDataDto);
+                }
+            }   
 
             SetEntityTags(entity, _tagRepository);
 
@@ -506,6 +524,56 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             return Database.ExecuteScalar<int>(fullSql);
         }
 
+        /// <inheritdoc />
+        public void SetLastLogin(string username, DateTime date)
+        {
+            // Important - these queries are designed to execute without an exclusive WriteLock taken in our distributed lock
+            // table. However due to the data that we are updating which relies on version data we cannot update this data
+            // without taking some locks, otherwise we'll end up with strange situations because when a member is updated, that operation
+            // deletes and re-inserts all property data. So if there are concurrent transactions, one deleting and re-inserting and another trying
+            // to update there can be problems. This is only an issue for cmsPropertyData, not umbracoContentVersion because that table just
+            // maintains a single row and it isn't deleted/re-inserted.
+            // So the important part here is the ForUpdate() call on the select to fetch the property data to update.
+
+            // Update the cms property value for the member
+
+            var sqlSelectTemplateProperty = SqlContext.Templates.Get("Umbraco.Core.MemberRepository.SetLastLogin1", s => s
+                .Select<PropertyDataDto>(x => x.Id)
+                .From<PropertyDataDto>()
+                .InnerJoin<PropertyTypeDto>().On<PropertyTypeDto, PropertyDataDto>((l, r) => l.Id == r.PropertyTypeId)
+                .InnerJoin<ContentVersionDto>().On<ContentVersionDto, PropertyDataDto>((l, r) => l.Id == r.VersionId)
+                .InnerJoin<NodeDto>().On<NodeDto, ContentVersionDto>((l, r) => l.NodeId == r.NodeId)
+                .InnerJoin<MemberDto>().On<MemberDto, NodeDto>((l, r) => l.NodeId == r.NodeId)
+                .Where<NodeDto>(x => x.NodeObjectType == SqlTemplate.Arg<Guid>("nodeObjectType"))
+                .Where<PropertyTypeDto>(x => x.Alias == SqlTemplate.Arg<string>("propertyTypeAlias"))
+                .Where<MemberDto>(x => x.LoginName == SqlTemplate.Arg<string>("username"))
+                .ForUpdate());
+            var sqlSelectProperty = sqlSelectTemplateProperty.Sql(Constants.ObjectTypes.Member, Constants.Conventions.Member.LastLoginDate, username);
+
+            var update = Sql()
+                .Update<PropertyDataDto>(u => u
+                    .Set(x => x.DateValue, date))
+                .WhereIn<PropertyDataDto>(x => x.Id, sqlSelectProperty);
+
+            Database.Execute(update);
+
+            // Update the umbracoContentVersion value for the member
+
+            var sqlSelectTemplateVersion = SqlContext.Templates.Get("Umbraco.Core.MemberRepository.SetLastLogin2", s => s
+               .Select<ContentVersionDto>(x => x.Id)
+               .From<ContentVersionDto>()               
+               .InnerJoin<NodeDto>().On<NodeDto, ContentVersionDto>((l, r) => l.NodeId == r.NodeId)
+               .InnerJoin<MemberDto>().On<MemberDto, NodeDto>((l, r) => l.NodeId == r.NodeId)
+               .Where<NodeDto>(x => x.NodeObjectType == SqlTemplate.Arg<Guid>("nodeObjectType"))
+               .Where<MemberDto>(x => x.LoginName == SqlTemplate.Arg<string>("username")));
+            var sqlSelectVersion = sqlSelectTemplateVersion.Sql(Constants.ObjectTypes.Member, username);
+
+            Database.Execute(Sql()
+                .Update<ContentVersionDto>(u => u
+                    .Set(x => x.VersionDate, date))
+                .WhereIn<ContentVersionDto>(x => x.Id, sqlSelectVersion));
+        }
+
         /// <summary>
         /// Gets paged member results.
         /// </summary>
@@ -529,20 +597,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
                 ordering);
         }
 
-        private string _pagedResultsByQueryWhere;
-
-        private string GetPagedResultsByQueryWhere()
-        {
-            if (_pagedResultsByQueryWhere == null)
-                _pagedResultsByQueryWhere = " AND ("
-                    + $"({SqlSyntax.GetQuotedTableName("umbracoNode")}.{SqlSyntax.GetQuotedColumnName("text")} LIKE @0)"
-                    + " OR "
-                    + $"({SqlSyntax.GetQuotedTableName("cmsMember")}.{SqlSyntax.GetQuotedColumnName("LoginName")} LIKE @0)"
-                    + ")";
-
-            return _pagedResultsByQueryWhere;
-        }
-
         protected override string ApplySystemOrdering(ref Sql<ISqlContext> sql, Ordering ordering)
         {
             if (ordering.OrderBy.InvariantEquals("email"))
@@ -632,5 +686,22 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
             member.ResetDirtyProperties(false);
             return member;
         }
+
+        public IMember GetByUsername(string username)
+        {
+            return _memberByUsernameCachePolicy.Get(username, PerformGetByUsername, PerformGetAllByUsername);
+        }
+
+        private IMember PerformGetByUsername(string username)
+        {
+            var query = Query<IMember>().Where(x => x.Username.Equals(username));
+            return PerformGetByQuery(query).FirstOrDefault();
+        }
+
+        private IEnumerable<IMember> PerformGetAllByUsername(params string[] usernames)
+        {
+            var query = Query<IMember>().WhereIn(x => x.Username, usernames);
+            return PerformGetByQuery(query);
+        }
     }
 }

src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimpleGetRepository.cs

@@ -11,6 +11,8 @@ using Umbraco.Core.Scoping;
 
 namespace Umbraco.Core.Persistence.Repositories.Implement
 {
+    // TODO: Obsolete this, change all implementations of this like in Dictionary to just use custom Cache policies like in the member repository.
+
     /// <summary>
     /// Simple abstract ReadOnly repository used to simply have PerformGet and PeformGetAll with an underlying cache
     /// </summary>

src/Umbraco.Infrastructure/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;