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 ce8c27e474..adfce25cce 100644 --- a/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs @@ -333,31 +333,15 @@ namespace Umbraco.Core.Persistence.Repositories /// /// 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); - } - - /// - /// 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; @@ -369,11 +353,11 @@ namespace Umbraco.Core.Persistence.Repositories var factory = new UmbracoEntityFactory(); - if (isMedia && includePropertyData) + if (isMedia) { var wheres = query.GetWhereClauses().ToArray(); - var mediaSql = GetFullSqlForMedia(entitySql.Append(GetGroupBy(isContent, isMedia, false)), sql => + var mediaSql = GetFullSqlForMedia(entitySql.Append(GetGroupBy(isContent, true, false)), sql => { //adds the additional filters foreach (var whereClause in wheres) @@ -389,20 +373,47 @@ 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, 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(); + 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 diff --git a/src/Umbraco.Core/Services/EntityService.cs b/src/Umbraco.Core/Services/EntityService.cs index ef316409a1..1e44bec909 100644 --- a/src/Umbraco.Core/Services/EntityService.cs +++ b/src/Umbraco.Core/Services/EntityService.cs @@ -285,39 +285,36 @@ namespace Umbraco.Core.Services /// An enumerable list of objects public virtual IEnumerable GetChildren(int parentId, UmbracoObjectTypes umbracoObjectType) { - return GetChildrenInternal(parentId, umbracoObjectType, true); + var objectTypeId = umbracoObjectType.GetGuid(); + 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 - /// UmbracoObjectType of the children to retrieve /// An enumerable list of objects - internal IEnumerable GetChildrenWithoutPropertyData(int parentId, UmbracoObjectTypes umbracoObjectType) + internal IEnumerable GetMediaChildrenWithoutPropertyData(int parentId) { - return GetChildrenInternal(parentId, umbracoObjectType, false); - } - - internal IEnumerable GetChildrenInternal(int parentId, UmbracoObjectTypes umbracoObjectType, bool includePropertyData) - { - var objectTypeId = umbracoObjectType.GetGuid(); + 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); - 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); + return ((EntityRepository)repository).GetMediaByQueryWithoutPropertyData(query); } } - + /// /// Returns a paged collection of children /// 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 ec0f0bf5aa..006355eb35 100644 --- a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs +++ b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs @@ -202,13 +202,16 @@ namespace Umbraco.Web.Trees entityId = entity.Id; } - // 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(); + 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 +}