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

Reduce sql queries when rendering blocklists in the content panel (#10521)

Browse Source 
Co-authored-by: Nikolaj <nel@umbraco.dk>
This commit is contained in:
Mole
2021-06-28 09:32:42 +02:00
committed by GitHub
parent ea1091db7c
commit 83ee9b4699
4 changed files with 141 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
src/Umbraco.Core/Services/IContentService.cs
View File

@@ -323,7 +323,7 @@ namespace Umbraco.Core.Services
/// <summary>
/// Empties the Recycle Bin by deleting all <see cref="IContent"/> that resides in the bin
/// </summary>
/// <param name="userId">Optional Id of the User emptying the Recycle Bin</param>
/// <param name="userId">Optional Id of the User emptying the Recycle Bin</param>
OperationResult EmptyRecycleBin(int userId = Constants.Security.SuperUserId);
/// <summary>
@@ -499,6 +499,11 @@ namespace Umbraco.Core.Services
/// </summary>
IContent Create(string name, int parentId, string documentTypeAlias, int userId = Constants.Security.SuperUserId);
/// <summary>
/// Creates a document
/// </summary>
IContent Create(string name, int parentId, IContentType contentType, int userId = Constants.Security.SuperUserId);
/// <summary>
/// Creates a document.
/// </summary>

31
src/Umbraco.Core/Services/Implement/ContentService.cs
View File

@@ -188,11 +188,34 @@ namespace Umbraco.Core.Services.Implement
// TODO: what about culture?
var contentType = GetContentType(contentTypeAlias);
if (contentType == null)
throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias));
return Create(name, parentId, contentType, userId);
}
/// <summary>
/// Creates an <see cref="IContent"/> object of a specified content type.
/// </summary>
/// <remarks>This method simply returns a new, non-persisted, IContent without any identity. It
/// is intended as a shortcut to creating new content objects that does not invoke a save
/// operation against the database.
/// </remarks>
/// <param name="name">The name of the content object.</param>
/// <param name="parentId">The identifier of the parent, or -1.</param>
/// <param name="contentType">The content type of the content</param>
/// <param name="userId">The optional id of the user creating the content.</param>
/// <returns>The content object.</returns>
public IContent Create(string name, int parentId, IContentType contentType,
int userId = Constants.Security.SuperUserId)
{
if (contentType is null)
{
throw new ArgumentException("Content type must be specified", nameof(contentType));
}
var parent = parentId > 0 ? GetById(parentId) : null;
if (parentId > 0 && parent == null)
if (parentId > 0 && parent is null)
{
throw new ArgumentException("No content with that id.", nameof(parentId));
}
var content = new Content(name, parentId, contentType);
using (var scope = ScopeProvider.CreateScope())
@@ -1088,7 +1111,7 @@ namespace Umbraco.Core.Services.Implement
/// <remarks>
/// <para>
/// Business logic cases such: as unpublishing a mandatory culture, or unpublishing the last culture, checking for pending scheduled publishing, etc... is dealt with in this method.
/// There is quite a lot of cases to take into account along with logic that needs to deal with scheduled saving/publishing, branch saving/publishing, etc...
/// There is quite a lot of cases to take into account along with logic that needs to deal with scheduled saving/publishing, branch saving/publishing, etc...
/// </para>
/// </remarks>
private PublishResult CommitDocumentChangesInternal(IScope scope, IContent content,

94
src/Umbraco.Web/Editors/ContentController.cs
View File

@@ -37,7 +37,7 @@ using Umbraco.Core.Models.Entities;
using Umbraco.Core.Persistence;
using Umbraco.Core.Security;
using Umbraco.Web.Routing;
using Umbraco.Core.Collections;
using Umbraco.Core.Mapping;
using Umbraco.Core.Scoping;
namespace Umbraco.Web.Editors
@@ -388,20 +388,65 @@ namespace Umbraco.Web.Editors
}
}
private ContentItemDisplay GetEmpty(IContentType contentType, int parentId)
private ContentItemDisplay CleanContentItemDisplay(ContentItemDisplay display)
{
var emptyContent = Services.ContentService.Create("", parentId, contentType.Alias, Security.GetUserId().ResultOr(0));
var mapped = MapToDisplay(emptyContent);
// translate the content type name if applicable
mapped.ContentTypeName = Services.TextService.UmbracoDictionaryTranslate(mapped.ContentTypeName);
display.ContentTypeName = Services.TextService.UmbracoDictionaryTranslate(display.ContentTypeName);
// if your user type doesn't have access to the Settings section it would not get this property mapped
if (mapped.DocumentType != null)
mapped.DocumentType.Name = Services.TextService.UmbracoDictionaryTranslate(mapped.DocumentType.Name);
if (display.DocumentType != null)
display.DocumentType.Name = Services.TextService.UmbracoDictionaryTranslate(display.DocumentType.Name);
//remove the listview app if it exists
mapped.ContentApps = mapped.ContentApps.Where(x => x.Alias != "umbListView").ToList();
display.ContentApps = display.ContentApps.Where(x => x.Alias != "umbListView").ToList();
return display;
}
return mapped;
private ContentItemDisplay GetEmpty(IContentType contentType, int parentId)
{
var emptyContent = Services.ContentService.Create("", parentId, contentType, Security.GetUserId().ResultOr(0));
var mapped = MapToDisplay(emptyContent);
return CleanContentItemDisplay(mapped);
}
/// <summary>
/// Gets an empty <see cref="ContentItemDisplay"/> for each content type in the IEnumerable, all with the same parent ID
/// </summary>
/// <remarks>Will attempt to re-use the same permissions for every content as long as the path and user are the same</remarks>
/// <param name="contentTypes"></param>
/// <param name="parentId"></param>
/// <returns></returns>
private IEnumerable<ContentItemDisplay> GetEmpties(IEnumerable<IContentType> contentTypes, int parentId)
{
var result = new List<ContentItemDisplay>();
var userId = Security.GetUserId().ResultOr(0);
var currentUser = Security.CurrentUser;
// We know that if the ID is less than 0 the parent is null.
// Since this is called with parent ID it's safe to assume that the parent is the same for all the content types.
var parent = parentId > 0 ? Services.ContentService.GetById(parentId) : null;
// Since the parent is the same and the path used to get permissions is based on the parent we only have to do it once
var path = parent == null ? "-1" : parent.Path;
var permissions = new Dictionary<string, EntityPermissionSet>
{
[path] = Services.UserService.GetPermissionsForPath(currentUser, path)
};
foreach (var contentType in contentTypes)
{
var emptyContent = Services.ContentService.Create("", parentId, contentType, userId);
var mapped = MapToDisplay(emptyContent, context =>
{
// Since the permissions depend on current user and path, we add both of these to context as well,
// that way we can compare the path and current user when mapping, if they're the same just take permissions
// and skip getting them again, in theory they should always be the same, but better safe than sorry.,
context.Items["Parent"] = parent;
context.Items["CurrentUser"] = currentUser;
context.Items["Permissions"] = permissions;
});
result.Add(CleanContentItemDisplay(mapped));
}
return result;
}
/// <summary>
@@ -412,22 +457,9 @@ namespace Umbraco.Web.Editors
[OutgoingEditorModelEvent]
public IDictionary<Guid, ContentItemDisplay> GetEmptyByKeys([FromUri] Guid[] contentTypeKeys, [FromUri] int parentId)
{
var result = new Dictionary<Guid, ContentItemDisplay>();
using var scope = _scopeProvider.CreateScope(autoComplete: true);
var contentTypes = Services.ContentTypeService.GetAll(contentTypeKeys).ToList();
foreach (var contentType in contentTypes)
{
if (contentType is null)
{
throw new HttpResponseException(HttpStatusCode.NotFound);
}
result.Add(contentType.Key, GetEmpty(contentType, parentId));
}
return result;
return GetEmpties(contentTypes, parentId).ToDictionary(x => x.ContentTypeKey);
}
[OutgoingEditorModelEvent]
@@ -2274,12 +2306,22 @@ namespace Umbraco.Web.Editors
/// </summary>
/// <param name="content"></param>
/// <returns></returns>
private ContentItemDisplay MapToDisplay(IContent content)
{
var display = Mapper.Map<ContentItemDisplay>(content, context =>
private ContentItemDisplay MapToDisplay(IContent content) =>
MapToDisplay(content, context =>
{
context.Items["CurrentUser"] = Security.CurrentUser;
});
/// <summary>
/// Used to map an <see cref="IContent"/> instance to a <see cref="ContentItemDisplay"/> and ensuring AllowPreview is set correctly.
/// Also allows you to pass in an action for the mapper context where you can pass additional information on to the mapper.
/// </summary>
/// <param name="content"></param>
/// <param name="contextOptions"></param>
/// <returns></returns>
private ContentItemDisplay MapToDisplay(IContent content, Action<MapperContext> contextOptions)
{
var display = Mapper.Map<ContentItemDisplay>(content, contextOptions);
display.AllowPreview = display.AllowPreview && content.Trashed == false && content.ContentType.IsElement == false;
return display;
}

48
src/Umbraco.Web/Models/Mapping/ContentMapDefinition.cs
View File

@@ -86,7 +86,20 @@ namespace Umbraco.Web.Models.Mapping
// Umbraco.Code.MapAll -AllowPreview -Errors -PersistedContent
private void Map(IContent source, ContentItemDisplay target, MapperContext context)
{
target.AllowedActions = GetActions(source);
// Both GetActions and DetermineIsChildOfListView use parent, so get it once here
// Parent might already be in context, so check there before using content service
IContent parent;
if (context.Items.TryGetValue("Parent", out var parentObj) &&
parentObj is IContent typedParent)
{
parent = typedParent;
}
else
{
parent = _contentService.GetParent(source);
}
target.AllowedActions = GetActions(source, parent, context);
target.AllowedTemplates = GetAllowedTemplates(source);
target.ContentApps = _commonMapper.GetContentApps(source);
target.ContentTypeId = source.ContentType.Id;
@@ -97,7 +110,7 @@ namespace Umbraco.Web.Models.Mapping
target.Icon = source.ContentType.Icon;
target.Id = source.Id;
target.IsBlueprint = source.Blueprint;
target.IsChildOfListView = DetermineIsChildOfListView(source, context);
target.IsChildOfListView = DetermineIsChildOfListView(source, parent, context);
target.IsContainer = source.ContentType.IsContainer;
target.IsElement = source.ContentType.IsElement;
target.Key = source.Key;
@@ -156,7 +169,7 @@ namespace Umbraco.Web.Models.Mapping
target.VariesByCulture = source.ContentType.VariesByCulture();
}
private IEnumerable<string> GetActions(IContent source)
private IEnumerable<string> GetActions(IContent source, IContent parent, MapperContext context)
{
var umbracoContext = _umbracoContextAccessor.UmbracoContext;
@@ -169,10 +182,24 @@ namespace Umbraco.Web.Models.Mapping
path = source.Path;
else
{
var parent = _contentService.GetById(source.ParentId);
path = parent == null ? "-1" : parent.Path;
}
// A bit of a mess, but we need to ensure that all the required values are here AND that they're the right type.
if (context.Items.TryGetValue("CurrentUser", out var userObject) &&
context.Items.TryGetValue("Permissions", out var permissionsObject) &&
userObject is IUser currentUser &&
permissionsObject is Dictionary<string, EntityPermissionSet> permissionsDict)
{
// If we already have permissions for a given path,
// and the current user is the same as was used to generate the permissions, return the stored permissions.
if (umbracoContext.Security.CurrentUser.Id == currentUser.Id &&
permissionsDict.TryGetValue(path, out var permissions))
{
return permissions.GetAllPermissions();
}
}
// TODO: This is certainly not ideal usage here - perhaps the best way to deal with this in the future is
// with the IUmbracoContextAccessor. In the meantime, if used outside of a web app this will throw a null
// reference exception :(
@@ -232,6 +259,7 @@ namespace Umbraco.Web.Models.Mapping
/// Checks if the content item is a descendant of a list view
/// </summary>
/// <param name="source"></param>
/// <param name="parent"></param>
/// <param name="context"></param>
/// <returns>
/// Returns true if the content item is a descendant of a list view and where the content is
@@ -243,7 +271,7 @@ namespace Umbraco.Web.Models.Mapping
/// false because the item is technically not being rendered as part of a list view but instead as a
/// real tree node. If we didn't perform this check then tree syncing wouldn't work correctly.
/// </remarks>
private bool DetermineIsChildOfListView(IContent source, MapperContext context)
private bool DetermineIsChildOfListView(IContent source, IContent parent, MapperContext context)
{
var userStartNodes = Array.Empty<int>();
@@ -258,12 +286,10 @@ namespace Umbraco.Web.Models.Mapping
// return false if this is the user's actual start node, the node will be rendered in the tree
// regardless of if it's a list view or not
if (userStartNodes.Contains(source.Id))
return false;
return false;
}
}
var parent = _contentService.GetParent(source);
if (parent == null)
return false;
@@ -297,6 +323,12 @@ namespace Umbraco.Web.Models.Mapping
private IDictionary<string, string> GetAllowedTemplates(IContent source)
{
// Element types can't have templates, so no need to query to get the content type
if (source.ContentType.IsElement)
{
return new Dictionary<string, string>();
}
var contentType = _contentTypeService.Get(source.ContentTypeId);
return contentType.AllowedTemplates