diff --git a/src/Umbraco.Core/Persistence/Factories/MediaFactory.cs b/src/Umbraco.Core/Persistence/Factories/MediaFactory.cs index 693aef594a..080a358657 100644 --- a/src/Umbraco.Core/Persistence/Factories/MediaFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MediaFactory.cs @@ -140,6 +140,9 @@ namespace Umbraco.Core.Persistence.Factories /// internal static bool TryMatch(string text, out string mediaPath) { + //TODO: In v8 we should allow exposing this via the property editor in a much nicer way so that the property editor + // can tell us directly what any URL is for a given property if it contains an asset + mediaPath = null; if (string.IsNullOrWhiteSpace(text)) @@ -155,4 +158,4 @@ namespace Umbraco.Core.Persistence.Factories return true; } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs index 40d35d2fee..adfce25cce 100644 --- a/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs @@ -331,14 +331,23 @@ 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) { - - bool isContent = objectTypeId == Constants.ObjectTypes.DocumentGuid || objectTypeId == Constants.ObjectTypes.DocumentBlueprintGuid; - bool isMedia = objectTypeId == Constants.ObjectTypes.MediaGuid; + 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(); @@ -364,22 +373,49 @@ 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)); - - //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)); - } - return collection.Select(x => x.BuildFromDynamic()).ToList(); + return GetByQueryInternal(entitySql, isContent, isMedia); } } + /// + /// 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 GetMediaByQueryWithoutPropertyData(IQuery query) + { + var sqlClause = GetBaseWhere(GetBase, false, true, null, UmbracoObjectTypes.Media.GetGuid()); + + var translator = new SqlTranslator(sqlClause, query); + var entitySql = translator.Translate(); + + return GetByQueryInternal(entitySql, false, true); + } + + internal IEnumerable GetByQueryInternal(Sql entitySql, bool isContent, bool isMedia) + { + var factory = new UmbracoEntityFactory(); + + //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, 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, isMedia)); + } + return collection.Select(x => x.BuildFromDynamic()).ToList(); + } + #endregion - + #region Sql Statements @@ -851,4 +887,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..1e44bec909 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 @@ -287,16 +286,35 @@ namespace Umbraco.Core.Services public virtual IEnumerable GetChildren(int parentId, UmbracoObjectTypes umbracoObjectType) { 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); + return repository.GetByQuery(query, objectTypeId); + } + } + + /// + /// Gets a collection of children by the parent's Id and UmbracoObjectType without adding property data + /// + /// Id of the parent to retrieve children for + /// An enumerable list of objects + internal IEnumerable GetMediaChildrenWithoutPropertyData(int parentId) + { + var objectTypeId = UmbracoObjectTypes.Media.GetGuid(); + 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; + // 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).GetMediaByQueryWithoutPropertyData(query); } } - + /// /// Returns a paged collection of children /// @@ -781,4 +799,4 @@ namespace Umbraco.Core.Services return node.NodeId; } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index 6cb13aa847..c3fe7df483 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -207,6 +207,9 @@ namespace Umbraco.Web.Trees return HasPathAccess(entity, queryStrings); } + internal override IEnumerable GetChildrenFromEntityService(int entityId) + => Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToList(); + /// /// Returns a collection of all menu items that can be on a content node /// diff --git a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs index 55a430326c..006355eb35 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,9 +202,16 @@ namespace Umbraco.Web.Trees entityId = entity.Id; } - return Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToList(); + return GetChildrenFromEntityService(entityId); } + /// + /// Abstract method to fetch the entities from the entity service + /// + /// + /// + internal abstract IEnumerable GetChildrenFromEntityService(int entityId); + /// /// Returns true or false if the current user has access to the node based on the user's allowed start node (path) access /// diff --git a/src/Umbraco.Web/Trees/MediaTreeController.cs b/src/Umbraco.Web/Trees/MediaTreeController.cs index 932ced9616..708357d66d 100644 --- a/src/Umbraco.Web/Trees/MediaTreeController.cs +++ b/src/Umbraco.Web/Trees/MediaTreeController.cs @@ -173,5 +173,12 @@ namespace Umbraco.Web.Trees { return _treeSearcher.ExamineSearch(Umbraco, query, UmbracoEntityTypes.Media, pageSize, pageIndex, out totalFound, searchFrom); } + + internal override IEnumerable GetChildrenFromEntityService(int entityId) + // 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 + => ((EntityService)Services.EntityService).GetMediaChildrenWithoutPropertyData(entityId).ToList(); } -} \ No newline at end of file +}