From f5fe07ffb7d924e28975452cbf5cecb9db93bab4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 14 Nov 2016 16:05:24 +0100 Subject: [PATCH] U4-6994 - more N+1 and refactoring --- .../Repositories/ContentRepository.cs | 210 ++++++------------ .../Repositories/MediaRepository.cs | 136 ++++++------ .../Repositories/MemberRepository.cs | 107 +++++---- 3 files changed, 201 insertions(+), 252 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 5d8818e1ac..de8d1bdd8b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -1,22 +1,14 @@ using System; using System.Collections.Generic; -using System.Data; using System.Globalization; using System.Linq; -using System.Linq.Expressions; -using System.Net.Http.Headers; -using System.Text; using System.Xml; using System.Xml.Linq; -using Umbraco.Core.Configuration; -using Umbraco.Core.Dynamics; -using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; - using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Querying; @@ -81,7 +73,7 @@ namespace Umbraco.Core.Persistence.Repositories var sql = GetBaseQuery(false); if (ids.Any()) { - sql.Where("umbracoNode.id in (@ids)", new { ids = ids }); + sql.Where("umbracoNode.id in (@ids)", new { ids }); } //we only want the newest ones with this method @@ -228,7 +220,7 @@ namespace Umbraco.Core.Persistence.Repositories private void RebuildXmlStructuresProcessQuery(Func serializer, IQuery query, Transaction tr, int pageSize) { var pageIndex = 0; - var total = long.MinValue; + long total; var processed = 0; do { @@ -239,7 +231,7 @@ namespace Umbraco.Core.Persistence.Repositories // see: http://issues.umbraco.org/issue/U4-6322 & http://issues.umbraco.org/issue/U4-5982 var descendants = GetPagedResultsByQuery(query, pageIndex, pageSize, out total, new Tuple("cmsDocument", "nodeId"), - ProcessQuery, "Path", Direction.Ascending, true); + sql => ProcessQuery(sql), "Path", Direction.Ascending, true); var xmlItems = (from descendant in descendants let xml = serializer(descendant) @@ -259,7 +251,7 @@ namespace Umbraco.Core.Persistence.Repositories var sql = GetBaseQuery(false) .Where(GetBaseWhereClause(), new { Id = id }) .OrderByDescending(x => x.VersionDate, SqlSyntax); - return GetAllBySql(sql); + return ProcessQuery(sql, true); } public override IContent GetByVersion(Guid versionId) @@ -670,63 +662,7 @@ namespace Umbraco.Core.Persistence.Repositories .OrderBy(x => x.Level, SqlSyntax) .OrderBy(x => x.SortOrder, SqlSyntax); - return GetAllBySql(sql); - } - - private IEnumerable GetAllBySql(Sql sql) - { - var dtos = Database.Fetch(sql); - var content = new IContent[dtos.Count]; - var defs = new List(); - - for (var i = 0; i < dtos.Count; i++) - { - var dto = dtos[i]; - - // if the cache contains the published version, use it - var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); - if (cached != null && cached.Published) - { - content[i] = cached; - continue; - } - - // else, need to fetch the version from the database - var contentType = _contentTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); - var factory = new ContentFactory(contentType, NodeObjectTypeId, dto.NodeId); - var c = factory.BuildEntity(dto); - if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) - c.Template = _templateRepository.Get(dto.TemplateId.Value); - content[i] = c; - - defs.Add(new DocumentDefinition( - dto.NodeId, - dto.VersionId, - dto.ContentVersionDto.VersionDate, - dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, - contentType - )); - } - - // going to load all properties for all docs from database, - // but at least in one queries thus avoiding N+1 - var propertyData = GetPropertyCollection(sql, defs); - - var dtoIndex = 0; - var defIndex = 0; - while (true) - { - if (defIndex == defs.Count) return content; - while (dtoIndex < dtos.Count && dtos[dtoIndex].NodeId != defs[defIndex].Id) dtoIndex++; - var cc = content[dtoIndex]; - cc.Properties = propertyData[cc.Id]; - - //on initial construction we don't want to have dirty properties tracked - // http://issues.umbraco.org/issue/U4-1946 - ((Entity)cc).ResetDirtyProperties(false); - - defIndex++; - } + return ProcessQuery(sql, true); } /// @@ -902,7 +838,7 @@ order by umbracoNode.level, umbracoNode.parentID, umbracoNode.sortOrder"; return GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, new Tuple("cmsDocument", "nodeId"), - ProcessQuery, orderBy, orderDirection, orderBySystemField, + sql => ProcessQuery(sql), orderBy, orderDirection, orderBySystemField, filterCallback); } @@ -933,83 +869,79 @@ order by umbracoNode.level, umbracoNode.parentID, umbracoNode.sortOrder"; return base.GetDatabaseFieldNameForOrderBy(orderBy); } - private IEnumerable ProcessQuery(Sql sql) + private IEnumerable ProcessQuery(Sql sql, bool withCache = false) { - //NOTE: This doesn't allow properties to be part of the query + // fetch returns a list so it's ok to iterate it in this method var dtos = Database.Fetch(sql); + if (dtos.Count == 0) return Enumerable.Empty(); - //nothing found - if (dtos.Any() == false) return Enumerable.Empty(); + var content = new IContent[dtos.Count]; + var defs = new List(); + var templateIds = new List(); - //content types - //NOTE: This should be ok for an SQL 'IN' statement, there shouldn't be an insane amount of content types - var contentTypes = _contentTypeRepository.GetAll(dtos.Select(x => x.ContentVersionDto.ContentDto.ContentTypeId).ToArray()) - .ToArray(); - - - var ids = dtos - .Where(dto => dto.TemplateId.HasValue && dto.TemplateId.Value > 0) - .Select(x => x.TemplateId.Value).ToArray(); - - //NOTE: This should be ok for an SQL 'IN' statement, there shouldn't be an insane amount of content types - var templates = ids.Length == 0 ? Enumerable.Empty() : _templateRepository.GetAll(ids).ToArray(); - - var dtosWithContentTypes = dtos - //This select into and null check are required because we don't have a foreign damn key on the contentType column - // http://issues.umbraco.org/issue/U4-5503 - .Select(x => new { dto = x, contentType = contentTypes.FirstOrDefault(ct => ct.Id == x.ContentVersionDto.ContentDto.ContentTypeId) }) - .Where(x => x.contentType != null) - .ToArray(); - - //Go get the property data for each document - var docDefs = dtosWithContentTypes.Select(d => new DocumentDefinition( - d.dto.NodeId, - d.dto.VersionId, - d.dto.ContentVersionDto.VersionDate, - d.dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, - d.contentType)); - - var propertyData = GetPropertyCollection(sql, docDefs); - - return dtosWithContentTypes.Select(d => CreateContentFromDto( - d.dto, - contentTypes.First(ct => ct.Id == d.dto.ContentVersionDto.ContentDto.ContentTypeId), - templates.FirstOrDefault(tem => tem.Id == (d.dto.TemplateId.HasValue ? d.dto.TemplateId.Value : -1)), - propertyData[d.dto.NodeId])); - } - - /// - /// Private method to create a content object from a DocumentDto, which is used by Get and GetByVersion. - /// - /// - /// - /// - /// - /// - private IContent CreateContentFromDto(DocumentDto dto, - IContentType contentType, - ITemplate template, - Models.PropertyCollection propCollection) - { - var factory = new ContentFactory(contentType, NodeObjectTypeId, dto.NodeId); - var content = factory.BuildEntity(dto); - - //Check if template id is set on DocumentDto, and get ITemplate if it is. - if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) + for (var i = 0; i < dtos.Count; i++) { - content.Template = template ?? _templateRepository.Get(dto.TemplateId.Value); - } - else - { - //ensure there isn't one set. - content.Template = null; + var dto = dtos[i]; + + // if the cache contains the published version, use it + if (withCache) + { + var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); + if (cached != null && cached.Published) + { + content[i] = cached; + continue; + } + } + + // else, need to fetch from the database + // content type repository is full-cache so OK to get each one independently + var contentType = _contentTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); + var factory = new ContentFactory(contentType, NodeObjectTypeId, dto.NodeId); + content[i] = factory.BuildEntity(dto); + + // need template + if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) + templateIds.Add(dto.TemplateId.Value); + + // need properties + defs.Add(new DocumentDefinition( + dto.NodeId, + dto.VersionId, + dto.ContentVersionDto.VersionDate, + dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, + contentType + )); } - content.Properties = propCollection; + // load all required templates in 1 query + var templates = _templateRepository.GetAll(templateIds.ToArray()) + .ToDictionary(x => x.Id, x => x); + + // load all properties for all documents from database in 1 query + var propertyData = GetPropertyCollection(sql, defs); + + // assign + var dtoIndex = 0; + foreach (var def in defs) + { + // move to corresponding item (which has to exist) + while (dtos[dtoIndex].NodeId != def.Id) dtoIndex++; + + // complete the item + var cc = content[dtoIndex]; + var dto = dtos[dtoIndex]; + ITemplate template = null; + if (dto.TemplateId.HasValue) + templates.TryGetValue(dto.TemplateId.Value, out template); // else null + cc.Template = template; + cc.Properties = propertyData[cc.Id]; + + //on initial construction we don't want to have dirty properties tracked + // http://issues.umbraco.org/issue/U4-1946 + ((Entity) cc).ResetDirtyProperties(false); + } - //on initial construction we don't want to have dirty properties tracked - // http://issues.umbraco.org/issue/U4-1946 - ((Entity)content).ResetDirtyProperties(false); return content; } diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs index 598c9e912d..48372e5517 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs @@ -4,21 +4,17 @@ using System.Globalization; using System.Linq; using System.Text; using System.Xml.Linq; -using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; -using Umbraco.Core.Dynamics; -using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models.Rdbms; - +using Umbraco.Core.Cache; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; -using Umbraco.Core.Services; namespace Umbraco.Core.Persistence.Repositories { @@ -137,6 +133,74 @@ namespace Umbraco.Core.Persistence.Repositories #region Overrides of VersionableRepositoryBase + public override IEnumerable GetAllVersions(int id) + { + var sql = GetBaseQuery(false) + .Where(GetBaseWhereClause(), new { Id = id }) + .OrderByDescending(x => x.VersionDate, SqlSyntax); + return ProcessQuery(sql, true); + } + + private IEnumerable ProcessQuery(Sql sql, bool withCache = false) + { + // fetch returns a list so it's ok to iterate it in this method + var dtos = Database.Fetch(sql); + var content = new IMedia[dtos.Count]; + var defs = new List(); + + for (var i = 0; i < dtos.Count; i++) + { + var dto = dtos[i]; + + // if the cache contains the item, use it + if (withCache) + { + var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); + if (cached != null) + { + content[i] = cached; + continue; + } + } + + // else, need to fetch from the database + // content type repository is full-cache so OK to get each one independently + var contentType = _mediaTypeRepository.Get(dto.ContentDto.ContentTypeId); + var factory = new MediaFactory(contentType, NodeObjectTypeId, dto.NodeId); + content[i] = factory.BuildEntity(dto); + + // need properties + defs.Add(new DocumentDefinition( + dto.NodeId, + dto.VersionId, + dto.VersionDate, + dto.ContentDto.NodeDto.CreateDate, + contentType + )); + } + + // load all properties for all documents from database in 1 query + var propertyData = GetPropertyCollection(sql, defs); + + // assign + var dtoIndex = 0; + foreach (var def in defs) + { + // move to corresponding item (which has to exist) + while (dtos[dtoIndex].NodeId != def.Id) dtoIndex++; + + // complete the item + var cc = content[dtoIndex]; + cc.Properties = propertyData[cc.Id]; + + //on initial construction we don't want to have dirty properties tracked + // http://issues.umbraco.org/issue/U4-1946 + ((Entity) cc).ResetDirtyProperties(false); + } + + return content; + } + public override IMedia GetByVersion(Guid versionId) { var sql = GetBaseQuery(false); @@ -175,7 +239,7 @@ namespace Umbraco.Core.Persistence.Repositories var subQuery = new Sql() .Select("id") .From(SqlSyntax) - .Where(x => x.NodeObjectType == NodeObjectTypeId); + .Where(x => x.NodeObjectType == NodeObjectTypeId); var deleteSql = SqlSyntax.GetDeleteSubquery("cmsContentXml", "nodeId", subQuery); Database.Execute(deleteSql); @@ -191,7 +255,7 @@ namespace Umbraco.Core.Persistence.Repositories .Where(x => x.NodeObjectType == NodeObjectTypeId); var deleteSql = SqlSyntax.GetDeleteSubquery("cmsContentXml", "nodeId", subQuery); - Database.Execute(deleteSql); + Database.Execute(deleteSql); } //now insert the data, again if something fails here, the whole transaction is reversed @@ -460,67 +524,11 @@ namespace Umbraco.Core.Persistence.Repositories return GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, new Tuple("cmsContentVersion", "contentId"), - ProcessQuery, orderBy, orderDirection, orderBySystemField, + sql => ProcessQuery(sql), orderBy, orderDirection, orderBySystemField, filterCallback); } - private IEnumerable ProcessQuery(Sql sql) - { - //NOTE: This doesn't allow properties to be part of the query - var dtos = Database.Fetch(sql); - - var ids = dtos.Select(x => x.ContentDto.ContentTypeId).ToArray(); - - //content types - var contentTypes = ids.Length == 0 ? Enumerable.Empty() : _mediaTypeRepository.GetAll(ids).ToArray(); - - var dtosWithContentTypes = dtos - //This select into and null check are required because we don't have a foreign damn key on the contentType column - // http://issues.umbraco.org/issue/U4-5503 - .Select(x => new { dto = x, contentType = contentTypes.FirstOrDefault(ct => ct.Id == x.ContentDto.ContentTypeId) }) - .Where(x => x.contentType != null) - .ToArray(); - - //Go get the property data for each document - var docDefs = dtosWithContentTypes.Select(d => new DocumentDefinition( - d.dto.NodeId, - d.dto.VersionId, - d.dto.VersionDate, - d.dto.ContentDto.NodeDto.CreateDate, - d.contentType)) - .ToArray(); - - var propertyData = GetPropertyCollection(sql, docDefs); - - return dtosWithContentTypes.Select(d => CreateMediaFromDto( - d.dto, - contentTypes.First(ct => ct.Id == d.dto.ContentDto.ContentTypeId), - propertyData[d.dto.NodeId])); - } - - /// - /// Private method to create a media object from a ContentDto - /// - /// - /// - /// - /// - private IMedia CreateMediaFromDto(ContentVersionDto dto, - IMediaType contentType, - PropertyCollection propCollection) - { - var factory = new MediaFactory(contentType, NodeObjectTypeId, dto.NodeId); - var media = factory.BuildEntity(dto); - - media.Properties = propCollection; - - //on initial construction we don't want to have dirty properties tracked - // http://issues.umbraco.org/issue/U4-1946 - ((Entity)media).ResetDirtyProperties(false); - return media; - } - /// /// Private method to create a media object from a ContentDto /// diff --git a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs index 216fc223db..4d8b19e522 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs @@ -2,24 +2,19 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; -using System.Linq.Expressions; using System.Text; using System.Xml.Linq; -using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; -using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; - +using Umbraco.Core.Cache; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Querying; -using Umbraco.Core.Persistence.Relators; using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; -using Umbraco.Core.Dynamics; namespace Umbraco.Core.Persistence.Repositories { @@ -380,6 +375,14 @@ namespace Umbraco.Core.Persistence.Repositories #region Overrides of VersionableRepositoryBase + public override IEnumerable GetAllVersions(int id) + { + var sql = GetBaseQuery(false) + .Where(GetBaseWhereClause(), new { Id = id }) + .OrderByDescending(x => x.VersionDate, SqlSyntax); + return ProcessQuery(sql, true); + } + public void RebuildXmlStructures(Func serializer, int groupSize = 5000, IEnumerable contentTypeIds = null) { @@ -645,7 +648,7 @@ namespace Umbraco.Core.Persistence.Repositories return GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, new Tuple("cmsMember", "nodeId"), - ProcessQuery, orderBy, orderDirection, orderBySystemField, + sql => ProcessQuery(sql), orderBy, orderDirection, orderBySystemField, filterCallback); } @@ -685,59 +688,65 @@ namespace Umbraco.Core.Persistence.Repositories return base.GetEntityPropertyNameForOrderBy(orderBy); } - private IEnumerable ProcessQuery(Sql sql) + private IEnumerable ProcessQuery(Sql sql, bool withCache = false) { - //NOTE: This doesn't allow properties to be part of the query + // fetch returns a list so it's ok to iterate it in this method var dtos = Database.Fetch(sql); - var ids = dtos.Select(x => x.ContentVersionDto.ContentDto.ContentTypeId).ToArray(); + var content = new IMember[dtos.Count]; + var defs = new List(); - //content types - var contentTypes = ids.Length == 0 ? Enumerable.Empty() : _memberTypeRepository.GetAll(ids).ToArray(); + for (var i = 0; i < dtos.Count; i++) + { + var dto = dtos[i]; - var dtosWithContentTypes = dtos - //This select into and null check are required because we don't have a foreign damn key on the contentType column - // http://issues.umbraco.org/issue/U4-5503 - .Select(x => new { dto = x, contentType = contentTypes.FirstOrDefault(ct => ct.Id == x.ContentVersionDto.ContentDto.ContentTypeId) }) - .Where(x => x.contentType != null) - .ToArray(); + // if the cache contains the item, use it + if (withCache) + { + var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); + if (cached != null) + { + content[i] = cached; + continue; + } + } - //Go get the property data for each document - IEnumerable docDefs = dtosWithContentTypes.Select(d => new DocumentDefinition( - d.dto.NodeId, - d.dto.ContentVersionDto.VersionId, - d.dto.ContentVersionDto.VersionDate, - d.dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, - d.contentType)); + // else, need to fetch from the database + // content type repository is full-cache so OK to get each one independently + var contentType = _memberTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); + var factory = new MemberFactory(contentType, NodeObjectTypeId, dto.NodeId); + content[i] = factory.BuildEntity(dto); - var propertyData = GetPropertyCollection(sql, docDefs); + // need properties + defs.Add(new DocumentDefinition( + dto.NodeId, + dto.ContentVersionDto.VersionId, + dto.ContentVersionDto.VersionDate, + dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, + contentType + )); + } - return dtosWithContentTypes.Select(d => CreateMemberFromDto( - d.dto, - contentTypes.First(ct => ct.Id == d.dto.ContentVersionDto.ContentDto.ContentTypeId), - propertyData[d.dto.NodeId])); - } + // load all properties for all documents from database in 1 query + var propertyData = GetPropertyCollection(sql, defs); - /// - /// Private method to create a member object from a MemberDto - /// - /// - /// - /// - /// - private IMember CreateMemberFromDto(MemberDto dto, - IMemberType contentType, - PropertyCollection propCollection) - { - var factory = new MemberFactory(contentType, NodeObjectTypeId, dto.ContentVersionDto.NodeId); - var member = factory.BuildEntity(dto); + // assign + var dtoIndex = 0; + foreach (var def in defs) + { + // move to corresponding item (which has to exist) + while (dtos[dtoIndex].NodeId != def.Id) dtoIndex++; - member.Properties = propCollection; + // complete the item + var cc = content[dtoIndex]; + cc.Properties = propertyData[cc.Id]; - //on initial construction we don't want to have dirty properties tracked - // http://issues.umbraco.org/issue/U4-1946 - ((Entity)member).ResetDirtyProperties(false); - return member; + //on initial construction we don't want to have dirty properties tracked + // http://issues.umbraco.org/issue/U4-1946 + ((Entity)cc).ResetDirtyProperties(false); + } + + return content; } ///