Slight refactor and cleanup

This commit is contained in:
Shannon
2018-11-01 13:12:47 +11:00
parent c0109e0531
commit 1429053681
6 changed files with 80 additions and 56 deletions

View File

@@ -140,6 +140,9 @@ namespace Umbraco.Core.Persistence.Factories
/// <returns></returns>
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;
}
}
}
}

View File

@@ -333,31 +333,15 @@ namespace Umbraco.Core.Persistence.Repositories
/// <summary>
/// Gets entities by query.
/// </summary>
/// <param name="query"></param>
/// <param name="objectTypeId"></param>
/// <returns></returns>
/// <remarks>
/// 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.
/// </summary>
/// <param name="query"></param>
/// <param name="objectTypeId"></param>
/// <returns></returns>
/// </remarks>
public virtual IEnumerable<IUmbracoEntity> GetByQuery(IQuery<IUmbracoEntity> query, Guid objectTypeId)
{
return GetByQueryInternal(query, objectTypeId, true);
}
/// <summary>
/// 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.
/// </summary>
/// <param name="query"></param>
/// <param name="objectTypeId"></param>
/// <returns></returns>
internal IEnumerable<IUmbracoEntity> GetByQueryWithoutPropertyData(IQuery<IUmbracoEntity> query, Guid objectTypeId)
{
return GetByQueryInternal(query, objectTypeId, false);
}
internal IEnumerable<IUmbracoEntity> GetByQueryInternal(IQuery<IUmbracoEntity> 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<dynamic>(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);
}
}
/// <summary>
/// Gets entities by query without fetching property data.
/// </summary>
/// <param name="query"></param>
/// <param name="objectTypeId"></param>
/// <returns></returns>
/// <remarks>
/// This is supposed to be internal and can be used when getting all entities without paging, without causing
/// performance issues.
/// </remarks>
internal IEnumerable<IUmbracoEntity> GetMediaByQueryWithoutPropertyData(IQuery<IUmbracoEntity> query)
{
var sqlClause = GetBaseWhere(GetBase, false, true, null, UmbracoObjectTypes.Media.GetGuid());
var translator = new SqlTranslator<IUmbracoEntity>(sqlClause, query);
var entitySql = translator.Translate();
return GetByQueryInternal(entitySql, false, true);
}
internal IEnumerable<IUmbracoEntity> 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<dynamic>(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

View File

@@ -285,39 +285,36 @@ namespace Umbraco.Core.Services
/// <returns>An enumerable list of <see cref="IUmbracoEntity"/> objects</returns>
public virtual IEnumerable<IUmbracoEntity> 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<IUmbracoEntity>.Builder.Where(x => x.ParentId == parentId);
return repository.GetByQuery(query, objectTypeId);
}
}
/// <summary>
/// Gets a collection of children by the parent's Id and UmbracoObjectType without adding property data
/// </summary>
/// <param name="parentId">Id of the parent to retrieve children for</param>
/// <param name="umbracoObjectType">UmbracoObjectType of the children to retrieve</param>
/// <returns>An enumerable list of <see cref="IUmbracoEntity"/> objects</returns>
internal IEnumerable<IUmbracoEntity> GetChildrenWithoutPropertyData(int parentId, UmbracoObjectTypes umbracoObjectType)
internal IEnumerable<IUmbracoEntity> GetMediaChildrenWithoutPropertyData(int parentId)
{
return GetChildrenInternal(parentId, umbracoObjectType, false);
}
internal IEnumerable<IUmbracoEntity> 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<IUmbracoEntity>.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);
}
}
/// <summary>
/// Returns a paged collection of children
/// </summary>

View File

@@ -207,6 +207,9 @@ namespace Umbraco.Web.Trees
return HasPathAccess(entity, queryStrings);
}
internal override IEnumerable<IUmbracoEntity> GetChildrenFromEntityService(int entityId)
=> Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToList();
/// <summary>
/// Returns a collection of all menu items that can be on a content node
/// </summary>

View File

@@ -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);
}
/// <summary>
/// Abstract method to fetch the entities from the entity service
/// </summary>
/// <param name="entityId"></param>
/// <returns></returns>
internal abstract IEnumerable<IUmbracoEntity> GetChildrenFromEntityService(int entityId);
/// <summary>
/// Returns true or false if the current user has access to the node based on the user's allowed start node (path) access
/// </summary>

View File

@@ -173,5 +173,12 @@ namespace Umbraco.Web.Trees
{
return _treeSearcher.ExamineSearch(Umbraco, query, UmbracoEntityTypes.Media, pageSize, pageIndex, out totalFound, searchFrom);
}
internal override IEnumerable<IUmbracoEntity> 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();
}
}
}