yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Removes the remaining part of the EntityRepository that was loading in ALL properties for media which we don't want whatsoever which means some other code is cleaned up/removed

Browse Source
This commit is contained in:
Shannon
2019-06-26 13:31:04 +10:00
parent cf53ba363d
commit 23b8e1cce8
11 changed files with 27 additions and 373 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
src/Umbraco.Core/Models/UmbracoEntity.cs
View File

@@ -225,42 +225,5 @@ namespace Umbraco.Core.Models
return clone;
}
/// <summary>
/// A struction that can be contained in the additional data of an UmbracoEntity representing
/// a user defined property
/// </summary>
public class EntityProperty : IDeepCloneable
{
public string PropertyEditorAlias { get; set; }
public object Value { get; set; }
public object DeepClone()
{
//Memberwise clone on Entity will work since it doesn't have any deep elements
// for any sub class this will work for standard properties as well that aren't complex object's themselves.
var clone = MemberwiseClone();
return clone;
}
protected bool Equals(EntityProperty other)
{
return PropertyEditorAlias.Equals(other.PropertyEditorAlias) && string.Equals(Value, other.Value);
}
public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj)) return false;
if (ReferenceEquals(this, obj)) return true;
if (obj.GetType() != this.GetType()) return false;
return Equals((EntityProperty) obj);
}
public override int GetHashCode()
{
unchecked
{
return (PropertyEditorAlias.GetHashCode() * 397) ^ (Value != null ? Value.GetHashCode() : 0);
}
}
}
}
}
}

146
src/Umbraco.Core/Persistence/Repositories/EntityRepository.cs
View File

@@ -20,27 +20,22 @@ namespace Umbraco.Core.Persistence.Repositories
/// </remarks>
internal class EntityRepository : DisposableObjectSlim, IEntityRepository
{
private readonly IDatabaseUnitOfWork _work;
public EntityRepository(IDatabaseUnitOfWork work)
{
_work = work;
UnitOfWork = work;
}
/// <summary>
/// Returns the Unit of Work added to the repository
/// </summary>
protected internal IDatabaseUnitOfWork UnitOfWork
{
get { return _work; }
}
protected internal IDatabaseUnitOfWork UnitOfWork { get; }
/// <summary>
/// Internal for testing purposes
/// </summary>
internal Guid UnitKey
{
get { return (Guid)_work.Key; }
get { return (Guid)UnitOfWork.Key; }
}
#region Query Methods
@@ -69,75 +64,8 @@ namespace Umbraco.Core.Persistence.Repositories
IEnumerable<IUmbracoEntity> result;
if (isMedia)
{
//Treat media differently for now, as an Entity it will be returned with ALL of it's properties in the AdditionalData bag!
var pagedResult = _work.Database.Page<dynamic>(pageIndex + 1, pageSize, pagedSql);
var ids = pagedResult.Items.Select(x => (int) x.id).InGroupsOf(2000);
var entities = pagedResult.Items.Select(factory.BuildEntityFromDynamic).Cast<IUmbracoEntity>().ToList();
//Now we need to merge in the property data since we need paging and we can't do this the way that the big media query was working before
foreach (var idGroup in ids)
{
var propSql = GetPropertySql(Constants.ObjectTypes.Media)
.Where("contentNodeId IN (@ids)", new { ids = idGroup });
propSql = (orderDirection == Direction.Descending) ? propSql.OrderByDescending("contentNodeId") : propSql.OrderBy("contentNodeId");
//This does NOT fetch all data into memory in a list, this will read
// over the records as a data reader, this is much better for performance and memory,
// but it means that during the reading of this data set, nothing else can be read
// from SQL server otherwise we'll get an exception.
var allPropertyData = _work.Database.Query<dynamic>(propSql);
//keep track of the current property data item being enumerated
var propertyDataSetEnumerator = allPropertyData.GetEnumerator();
var hasCurrent = false; // initially there is no enumerator.Current
try
{
//This must be sorted by node id (which is done by SQL) because this is how we are sorting the query to lookup property types above,
// which allows us to more efficiently iterate over the large data set of property values.
foreach (var entity in entities)
{
// assemble the dtos for this def
// use the available enumerator.Current if any else move to next
while (hasCurrent || propertyDataSetEnumerator.MoveNext())
{
if (propertyDataSetEnumerator.Current.contentNodeId == entity.Id)
{
hasCurrent = false; // enumerator.Current is not available
//the property data goes into the additional data
entity.AdditionalData[propertyDataSetEnumerator.Current.propertyTypeAlias] = new UmbracoEntity.EntityProperty
{
PropertyEditorAlias = propertyDataSetEnumerator.Current.propertyEditorAlias,
Value = StringExtensions.IsNullOrWhiteSpace(propertyDataSetEnumerator.Current.dataNtext)
? propertyDataSetEnumerator.Current.dataNvarchar
: StringExtensions.ConvertToJsonIfPossible(propertyDataSetEnumerator.Current.dataNtext)
};
}
else
{
hasCurrent = true; // enumerator.Current is available for another def
break; // no more propertyDataDto for this def
}
}
}
}
finally
{
propertyDataSetEnumerator.Dispose();
}
}
result = entities;
}
else
{
var pagedResult = _work.Database.Page<dynamic>(pageIndex + 1, pageSize, pagedSql);
result = pagedResult.Items.Select(factory.BuildEntityFromDynamic).Cast<IUmbracoEntity>().ToList();
}
var pagedResult = UnitOfWork.Database.Page<dynamic>(pageIndex + 1, pageSize, pagedSql);
result = pagedResult.Items.Select(factory.BuildEntityFromDynamic).Cast<IUmbracoEntity>().ToList();
//The total items from the PetaPoco page query will be wrong due to the Outer join used on parent, depending on the search this will
//return duplicate results when the COUNT is used in conjuction with it, so we need to get the total on our own.
@@ -159,7 +87,7 @@ namespace Umbraco.Core.Persistence.Repositories
var translatorCount = new SqlTranslator<IUmbracoEntity>(sqlCountClause, query);
var countSql = translatorCount.Translate();
totalRecords = _work.Database.ExecuteScalar<int>(countSql);
totalRecords = UnitOfWork.Database.ExecuteScalar<int>(countSql);
return result;
}
@@ -167,7 +95,7 @@ namespace Umbraco.Core.Persistence.Repositories
public IUmbracoEntity GetByKey(Guid key)
{
var sql = GetBaseWhere(GetBase, false, false, key);
var nodeDto = _work.Database.FirstOrDefault<dynamic>(sql);
var nodeDto = UnitOfWork.Database.FirstOrDefault<dynamic>(sql);
if (nodeDto == null)
return null;
@@ -187,7 +115,7 @@ namespace Umbraco.Core.Persistence.Repositories
var factory = new UmbracoEntityFactory();
//query = read forward data reader, do not load everything into mem
var dtos = _work.Database.Query<dynamic>(sql);
var dtos = UnitOfWork.Database.Query<dynamic>(sql);
var collection = new EntityDefinitionCollection();
foreach (var dto in dtos)
{
@@ -202,7 +130,7 @@ namespace Umbraco.Core.Persistence.Repositories
public virtual IUmbracoEntity Get(int id)
{
var sql = GetBaseWhere(GetBase, false, false, id);
var nodeDto = _work.Database.FirstOrDefault<dynamic>(sql);
var nodeDto = UnitOfWork.Database.FirstOrDefault<dynamic>(sql);
if (nodeDto == null)
return null;
@@ -222,7 +150,7 @@ namespace Umbraco.Core.Persistence.Repositories
var factory = new UmbracoEntityFactory();
//query = read forward data reader, do not load everything into mem
var dtos = _work.Database.Query<dynamic>(sql);
var dtos = UnitOfWork.Database.Query<dynamic>(sql);
var collection = new EntityDefinitionCollection();
foreach (var dto in dtos)
{
@@ -256,7 +184,7 @@ namespace Umbraco.Core.Persistence.Repositories
var factory = new UmbracoEntityFactory();
//query = read forward data reader, do not load everything into mem
var dtos = _work.Database.Query<dynamic>(sql);
var dtos = UnitOfWork.Database.Query<dynamic>(sql);
var collection = new EntityDefinitionCollection();
foreach (var dto in dtos)
{
@@ -283,7 +211,7 @@ namespace Umbraco.Core.Persistence.Repositories
{
var sql = new Sql("SELECT id, path FROM umbracoNode WHERE umbracoNode.nodeObjectType=@type", new { type = objectTypeId });
if (filter != null) filter(sql);
return _work.Database.Fetch<EntityPath>(sql);
return UnitOfWork.Database.Fetch<EntityPath>(sql);
}
public virtual IEnumerable<IUmbracoEntity> GetByQuery(IQuery<IUmbracoEntity> query)
@@ -292,7 +220,7 @@ namespace Umbraco.Core.Persistence.Repositories
var translator = new SqlTranslator<IUmbracoEntity>(sqlClause, query);
var sql = translator.Translate().Append(GetGroupBy(false, false));
var dtos = _work.Database.Fetch<dynamic>(sql);
var dtos = UnitOfWork.Database.Fetch<dynamic>(sql);
var factory = new UmbracoEntityFactory();
var list = dtos.Select(factory.BuildEntityFromDynamic).Cast<IUmbracoEntity>().ToList();
@@ -306,10 +234,6 @@ namespace Umbraco.Core.Persistence.Repositories
/// <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.
/// </remarks>
public virtual IEnumerable<IUmbracoEntity> GetByQuery(IQuery<IUmbracoEntity> query, Guid objectTypeId)
{
var isContent = objectTypeId == Constants.ObjectTypes.DocumentGuid || objectTypeId == Constants.ObjectTypes.DocumentBlueprintGuid;
@@ -323,27 +247,7 @@ namespace Umbraco.Core.Persistence.Repositories
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)
private IEnumerable<IUmbracoEntity> GetByQueryInternal(Sql entitySql, bool isContent, bool isMedia)
{
var factory = new UmbracoEntityFactory();
@@ -351,7 +255,7 @@ namespace Umbraco.Core.Persistence.Repositories
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 dtos = UnitOfWork.Database.Query<dynamic>(finalSql);
var collection = new EntityDefinitionCollection();
foreach (var dto in dtos)
{
@@ -392,22 +296,6 @@ namespace Umbraco.Core.Persistence.Repositories
return entitySql.Append(GetGroupBy(isContent, true, false));
}
private Sql GetPropertySql(string nodeObjectType)
{
var sql = new Sql()
.Select("contentNodeId, versionId, dataNvarchar, dataNtext, propertyEditorAlias, alias as propertyTypeAlias")
.From<PropertyDataDto>()
.InnerJoin<NodeDto>()
.On<PropertyDataDto, NodeDto>(dto => dto.NodeId, dto => dto.NodeId)
.InnerJoin<PropertyTypeDto>()
.On<PropertyTypeDto, PropertyDataDto>(dto => dto.Id, dto => dto.PropertyTypeId)
.InnerJoin<DataTypeDto>()
.On<PropertyTypeDto, DataTypeDto>(dto => dto.DataTypeId, dto => dto.DataTypeId)
.Where("umbracoNode.nodeObjectType = @nodeObjectType", new { nodeObjectType = nodeObjectType });
return sql;
}
protected virtual Sql GetBase(bool isContent, bool isMedia, Action<Sql> customFilter)
{
return GetBase(isContent, isMedia, customFilter, false);
@@ -638,13 +526,13 @@ namespace Umbraco.Core.Persistence.Repositories
public bool Exists(Guid key)
{
var sql = new Sql().Select("COUNT(*)").From("umbracoNode").Where("uniqueID=@uniqueID", new {uniqueID = key});
return _work.Database.ExecuteScalar<int>(sql) > 0;
return UnitOfWork.Database.ExecuteScalar<int>(sql) > 0;
}
public bool Exists(int id)
{
var sql = new Sql().Select("COUNT(*)").From("umbracoNode").Where("id=@id", new { id = id });
return _work.Database.ExecuteScalar<int>(sql) > 0;
return UnitOfWork.Database.ExecuteScalar<int>(sql) > 0;
}
#region private classes

21
src/Umbraco.Core/Services/EntityService.cs
View File

@@ -234,27 +234,6 @@ namespace Umbraco.Core.Services
}
}
/// <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>
/// <returns>An enumerable list of <see cref="IUmbracoEntity"/> objects</returns>
internal IEnumerable<IUmbracoEntity> GetMediaChildrenWithoutPropertyData(int parentId)
{
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);
// 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);
}
}
/// <summary>
/// Returns a paged collection of children
/// </summary>

27
src/Umbraco.Tests/Models/UmbracoEntityTests.cs
View File

@@ -177,18 +177,7 @@ namespace Umbraco.Tests.Models
};
item.AdditionalData.Add("test1", 3);
item.AdditionalData.Add("test2", "valuie");
item.AdditionalData.Add("test3", new UmbracoEntity.EntityProperty()
{
Value = "test",
PropertyEditorAlias = "TestPropertyEditor"
});
item.AdditionalData.Add("test4", new UmbracoEntity.EntityProperty()
{
Value = "test2",
PropertyEditorAlias = "TestPropertyEditor2"
});
var clone = (UmbracoEntity)item.DeepClone();
Assert.AreNotSame(clone, item);
@@ -250,20 +239,10 @@ namespace Umbraco.Tests.Models
};
item.AdditionalData.Add("test1", 3);
item.AdditionalData.Add("test2", "valuie");
item.AdditionalData.Add("test3", new UmbracoEntity.EntityProperty()
{
Value = "test",
PropertyEditorAlias = "TestPropertyEditor"
});
item.AdditionalData.Add("test4", new UmbracoEntity.EntityProperty()
{
Value = "test2",
PropertyEditorAlias = "TestPropertyEditor2"
});
var result = ss.ToStream(item);
var json = result.ResultStream.ToJsonString();
Debug.Print(json);
}
}
}
}

3
src/Umbraco.Web/Trees/ContentTreeController.cs
View File

@@ -207,9 +207,6 @@ 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>

8
src/Umbraco.Web/Trees/ContentTreeControllerBase.cs
View File

@@ -205,12 +205,8 @@ namespace Umbraco.Web.Trees
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);
private IEnumerable<IUmbracoEntity> GetChildrenFromEntityService(int entityId)
=> Services.EntityService.GetChildren(entityId, UmbracoObjectType).ToList();
/// <summary>
/// Returns true or false if the current user has access to the node based on the user's allowed start node (path) access

8
src/Umbraco.Web/Trees/MediaTreeController.cs
View File

@@ -177,12 +177,6 @@ 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();
}
}

1
src/Umbraco.Web/Umbraco.Web.csproj
View File

@@ -1812,7 +1812,6 @@
<Compile Include="WebServices\DomainsApiController.cs" />
<Compile Include="WebServices\EmbedMediaService.cs" />
<Compile Include="WebServices\ExamineManagementApiController.cs" />
<Compile Include="WebServices\FolderBrowserService.cs" />
<Compile Include="WebServices\TagsController.cs" />
<Compile Include="WebServices\UmbracoAuthorizedHttpHandler.cs" />
<Compile Include="WebServices\SaveFileController.cs" />

114
src/Umbraco.Web/WebServices/FolderBrowserService.cs
View File

@@ -1,114 +0,0 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Web.Script.Serialization;
using Umbraco.Core;
using Umbraco.Core.IO;
using Umbraco.Core.Models;
using Umbraco.Web.Media.ThumbnailProviders;
using umbraco.BusinessLogic;
using umbraco.cms.businesslogic.Tags;
using Umbraco.Web.BaseRest;
using Tag = umbraco.cms.businesslogic.Tags.Tag;
namespace Umbraco.Web.WebServices
{
//TODO: Can we convert this to MVC please instead of /base?
[Obsolete("Thumbnails are generated by ImageProcessor, use that instead")]
[RestExtension("FolderBrowserService")]
public class FolderBrowserService
{
[RestExtensionMethod(ReturnXml = false)]
public static string GetChildren(int parentId)
{
var currentUser = GetCurrentUser();
AuthorizeAccess(parentId, currentUser);
// Get children and filter
var data = new List<object>();
var service = ApplicationContext.Current.Services.EntityService;
var entities = service.GetChildren(parentId, UmbracoObjectTypes.Media);
foreach (UmbracoEntity entity in entities)
{
var uploadFieldProperty = entity.AdditionalData
.Select(x => x.Value as UmbracoEntity.EntityProperty)
.Where(x => x != null)
.FirstOrDefault(x => x.PropertyEditorAlias == Constants.PropertyEditors.UploadFieldAlias);
//var uploadFieldProperty = entity.UmbracoProperties.FirstOrDefault(x => x.PropertyEditorAlias == Constants.PropertyEditors.UploadFieldAlias);
var thumbnailUrl = uploadFieldProperty == null ? "" : ThumbnailProvidersResolver.Current.GetThumbnailUrl((string)uploadFieldProperty.Value);
var item = new
{
Id = entity.Id,
Path = entity.Path,
Name = entity.Name,
Tags = string.Join(",", Tag.GetTags(entity.Id).Select(x => x.TagCaption)),
MediaTypeAlias = entity.ContentTypeAlias,
EditUrl = string.Format("editMedia.aspx?id={0}", entity.Id),
FileUrl = uploadFieldProperty == null
? ""
: uploadFieldProperty.Value,
ThumbnailUrl = string.IsNullOrEmpty(thumbnailUrl)
? IOHelper.ResolveUrl(string.Format("{0}/images/thumbnails/{1}", SystemDirectories.Umbraco, entity.ContentTypeThumbnail))
: thumbnailUrl
};
data.Add(item);
}
return new JavaScriptSerializer().Serialize(data);
}
[RestExtensionMethod(ReturnXml = false)]
public static string Delete(string nodeIds)
{
var currentUser = GetCurrentUser();
var nodeIdParts = nodeIds.Split(',');
foreach (var nodeIdPart in nodeIdParts.Where(x => string.IsNullOrEmpty(x) == false))
{
int nodeId;
if (Int32.TryParse(nodeIdPart, out nodeId) == false)
continue;
var node = new global::umbraco.cms.businesslogic.media.Media(nodeId);
AuthorizeAccess(node, currentUser);
node.delete(("," + node.Path + ",").Contains(",-21,"));
}
return new JavaScriptSerializer().Serialize(new { success = true });
}
private static User GetCurrentUser()
{
var currentUser = User.GetCurrent();
if (currentUser == null)
throw new UnauthorizedAccessException("You must be logged in to use this service");
return currentUser;
}
private static void AuthorizeAccess(global::umbraco.cms.businesslogic.media.Media mediaItem, User currentUser)
{
if (("," + mediaItem.Path + ",").Contains("," + currentUser.StartMediaId + ",") == false)
throw new UnauthorizedAccessException("You do not have access to this Media node");
}
private static void AuthorizeAccess(int parentId, User currentUser)
{
var service = ApplicationContext.Current.Services.EntityService;
var parentMedia = service.Get(parentId, UmbracoObjectTypes.Media);
var mediaPath = parentMedia == null ? parentId.ToString(CultureInfo.InvariantCulture) : parentMedia.Path;
if (("," + mediaPath + ",").Contains("," + currentUser.StartMediaId + ",") == false)
throw new UnauthorizedAccessException("You do not have access to this Media node");
}
}
}

31
src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseMediaTree.cs
View File

@@ -189,35 +189,8 @@ function openMedia(id) {
return "";
}
/// <summary>
/// NOTE: New implementation of the legacy GetLinkValue. This is however a bit quirky as a media item can have multiple "Linkable DataTypes".
/// Returns the value for a link in WYSIWYG mode, by default only media items that have a
/// DataTypeUploadField are linkable, however, a custom tree can be created which overrides
/// this method, or another GUID for a custom data type can be added to the LinkableMediaDataTypes
/// list on application startup.
/// </summary>
/// <param name="entity"></param>
/// <returns></returns>
internal virtual string GetLinkValue(UmbracoEntity entity)
{
foreach (var property in entity.AdditionalData
.Select(x => x.Value as UmbracoEntity.EntityProperty)
.Where(x => x != null))
{
//required for backwards compatibility with v7 with changing the GUID -> alias
var controlId = LegacyPropertyEditorIdToAliasConverter.GetLegacyIdFromAlias(property.PropertyEditorAlias, LegacyPropertyEditorIdToAliasConverter.NotFoundLegacyIdResponseBehavior.ReturnNull);
if (controlId != null)
{
if (LinkableMediaDataTypes.Contains(controlId.Value)
&& string.IsNullOrEmpty((string)property.Value) == false)
return property.Value.ToString();
}
}
return "";
}
[Obsolete("Just like this class is and is not used whatsoever for a very long time, this method will return an empty string")]
internal virtual string GetLinkValue(UmbracoEntity entity) => string.Empty;
/// <summary>
/// By default, any media type that is to be "linkable" in the WYSIWYG editor must contain

2
src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseTree.cs
View File

@@ -652,4 +652,4 @@ namespace umbraco.cms.presentation.Trees
}
}
}