From c4ee9abe68b2ea7012ff565277a2faf2cdd08c3a Mon Sep 17 00:00:00 2001 From: Claus Date: Sat, 27 Oct 2018 12:51:05 +0200 Subject: [PATCH] adds internal methods for getting media entities through the entity repository/service without fetching all property data. ensures the trees use this method to avoid fetching property data in the media tree. --- .../Repositories/EntityRepository.cs | 43 +++++++++++++++---- src/Umbraco.Core/Services/EntityService.cs | 33 +++++++++++--- .../Trees/ContentTreeControllerBase.cs | 9 ++-- 3 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs index 40d35d2fee..ca6fc36e23 100644 --- a/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs @@ -331,24 +331,49 @@ namespace Umbraco.Core.Persistence.Repositories return list; } + /// + /// Gets entities by query. + /// Note that this will also fetch all property data for media items, which can cause performance problems + /// when used without paging, in sites with large amounts of data in cmsPropertyData. + /// + /// + /// + /// public virtual IEnumerable GetByQuery(IQuery query, Guid objectTypeId) { + return GetByQueryInternal(query, objectTypeId, true); + } - bool isContent = objectTypeId == Constants.ObjectTypes.DocumentGuid || objectTypeId == Constants.ObjectTypes.DocumentBlueprintGuid; - bool isMedia = objectTypeId == Constants.ObjectTypes.MediaGuid; + /// + /// Gets entities by query without fetching property data. + /// This is supposed to be internal and can be used when getting all entities without paging, without causing + /// performance issues. + /// + /// + /// + /// + internal IEnumerable GetByQueryWithoutPropertyData(IQuery query, Guid objectTypeId) + { + return GetByQueryInternal(query, objectTypeId, false); + } + + internal IEnumerable GetByQueryInternal(IQuery query, Guid objectTypeId, bool includePropertyData) + { + var isContent = objectTypeId == Constants.ObjectTypes.DocumentGuid || objectTypeId == Constants.ObjectTypes.DocumentBlueprintGuid; + var isMedia = objectTypeId == Constants.ObjectTypes.MediaGuid; var sqlClause = GetBaseWhere(GetBase, isContent, isMedia, null, objectTypeId); - + var translator = new SqlTranslator(sqlClause, query); var entitySql = translator.Translate(); var factory = new UmbracoEntityFactory(); - if (isMedia) + if (isMedia && includePropertyData) { var wheres = query.GetWhereClauses().ToArray(); - var mediaSql = GetFullSqlForMedia(entitySql.Append(GetGroupBy(isContent, true, false)), sql => + var mediaSql = GetFullSqlForMedia(entitySql.Append(GetGroupBy(isContent, isMedia, false)), sql => { //adds the additional filters foreach (var whereClause in wheres) @@ -365,21 +390,21 @@ namespace Umbraco.Core.Persistence.Repositories else { //use dynamic so that we can get ALL properties from the SQL so we can chuck that data into our AdditionalData - var finalSql = entitySql.Append(GetGroupBy(isContent, false)); + var finalSql = entitySql.Append(GetGroupBy(isContent, isMedia)); //query = read forward data reader, do not load everything into mem var dtos = _work.Database.Query(finalSql); var collection = new EntityDefinitionCollection(); foreach (var dto in dtos) { - collection.AddOrUpdate(new EntityDefinition(factory, dto, isContent, false)); + collection.AddOrUpdate(new EntityDefinition(factory, dto, isContent, isMedia)); } return collection.Select(x => x.BuildFromDynamic()).ToList(); } } #endregion - + #region Sql Statements @@ -851,4 +876,4 @@ namespace Umbraco.Core.Persistence.Repositories } #endregion } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Services/EntityService.cs b/src/Umbraco.Core/Services/EntityService.cs index b660927df3..ef316409a1 100644 --- a/src/Umbraco.Core/Services/EntityService.cs +++ b/src/Umbraco.Core/Services/EntityService.cs @@ -2,8 +2,6 @@ using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; -using System.Text; -using Umbraco.Core.Cache; using Umbraco.Core.CodeAnnotations; using Umbraco.Core.Events; using Umbraco.Core.Logging; @@ -13,6 +11,7 @@ using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Querying; +using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.UnitOfWork; namespace Umbraco.Core.Services @@ -285,15 +284,37 @@ namespace Umbraco.Core.Services /// UmbracoObjectType of the children to retrieve /// An enumerable list of objects public virtual IEnumerable GetChildren(int parentId, UmbracoObjectTypes umbracoObjectType) + { + return GetChildrenInternal(parentId, umbracoObjectType, true); + } + + /// + /// Gets a collection of children by the parent's Id and UmbracoObjectType without adding property data + /// + /// Id of the parent to retrieve children for + /// UmbracoObjectType of the children to retrieve + /// An enumerable list of objects + internal IEnumerable GetChildrenWithoutPropertyData(int parentId, UmbracoObjectTypes umbracoObjectType) + { + return GetChildrenInternal(parentId, umbracoObjectType, false); + } + + internal IEnumerable GetChildrenInternal(int parentId, UmbracoObjectTypes umbracoObjectType, bool includePropertyData) { var objectTypeId = umbracoObjectType.GetGuid(); - using (var uow = UowProvider.GetUnitOfWork(readOnly:true)) + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateEntityRepository(uow); var query = Query.Builder.Where(x => x.ParentId == parentId); - var contents = repository.GetByQuery(query, objectTypeId); - return contents; + if (includePropertyData) + return repository.GetByQuery(query, objectTypeId); + + // Not pretty having to cast the repository, but it is the only way to get to use an internal method that we + // do not want to make public on the interface. Unfortunately also prevents this from being unit tested. + // See this issue for details on why we need this: + // https://github.com/umbraco/Umbraco-CMS/issues/3457 + return ((EntityRepository)repository).GetByQueryWithoutPropertyData(query, objectTypeId); } } @@ -781,4 +802,4 @@ namespace Umbraco.Core.Services return node.NodeId; } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs index 55a430326c..ec0f0bf5aa 100644 --- a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs +++ b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.ComponentModel; using System.Linq; using System.Net; using System.Net.Http; @@ -11,12 +10,12 @@ using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; -using Umbraco.Core.Persistence; using Umbraco.Web.Models.Trees; using Umbraco.Web.WebApi.Filters; using umbraco; using umbraco.BusinessLogic.Actions; using System.Globalization; +using Umbraco.Core.Services; namespace Umbraco.Web.Trees { @@ -203,7 +202,11 @@ namespace Umbraco.Web.Trees entityId = entity.Id; } - return Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToList(); + // Not pretty having to cast the service, but it is the only way to get to use an internal method that we + // do not want to make public on the interface. Unfortunately also prevents this from being unit tested. + // See this issue for details on why we need this: + // https://github.com/umbraco/Umbraco-CMS/issues/3457 + return ((EntityService)Services.EntityService).GetChildrenWithoutPropertyData(entityId, UmbracoObjectType).ToList(); } ///