From 87f09b2ee6c8edb09fa8feb4f497d1d9e15a804a Mon Sep 17 00:00:00 2001 From: leekelleher Date: Thu, 18 Jan 2018 10:49:18 +0000 Subject: [PATCH] U4-10845 MultiNodeTreePickerPropertyConverter - trashed content causes database lookups If the first ID in the MNTP value is a trashed content node, then the `ConvertSourceToObject` method would check if the ID is a media or member node, both of these incur a database call. This can cause a detrimental performance impact on the website. This patch uses the `startNode.type` prevalue to detect the node type, (e.g. Content, Media or Member). The prevalue is cached in a private `ConcurrentDictionary` and refreshed using the `DataTypeCacheRefresher` event. > _Following the same caching-pattern as the Slider, Tags and MediaPicker value-converters._ --- .../Cache/DataTypeCacheRefresher.cs | 1 + .../MultiNodeTreePickerPropertyConverter.cs | 113 ++++++++++++------ 2 files changed, 79 insertions(+), 35 deletions(-) diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index 65d366e2ad..d28fe5cf88 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -117,6 +117,7 @@ namespace Umbraco.Web.Cache LegacyMediaPickerPropertyConverter.ClearCaches(); SliderValueConverter.ClearCaches(); MediaPickerPropertyConverter.ClearCaches(); + MultiNodeTreePickerPropertyConverter.ClearCaches(); base.Refresh(jsonPayload); diff --git a/src/Umbraco.Web/PropertyEditors/ValueConverters/MultiNodeTreePickerPropertyConverter.cs b/src/Umbraco.Web/PropertyEditors/ValueConverters/MultiNodeTreePickerPropertyConverter.cs index 249b7e0cac..abe444435f 100644 --- a/src/Umbraco.Web/PropertyEditors/ValueConverters/MultiNodeTreePickerPropertyConverter.cs +++ b/src/Umbraco.Web/PropertyEditors/ValueConverters/MultiNodeTreePickerPropertyConverter.cs @@ -8,16 +8,18 @@ // -------------------------------------------------------------------------------------------------------------------- using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.Linq; +using Newtonsoft.Json; using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors; using Umbraco.Core.PropertyEditors.ValueConverters; -using Umbraco.Web.Extensions; +using Umbraco.Core.Services; namespace Umbraco.Web.PropertyEditors.ValueConverters { @@ -32,6 +34,20 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters [PropertyValueCache(PropertyCacheValue.XPath, PropertyCacheLevel.Content)] public class MultiNodeTreePickerPropertyConverter : PropertyValueConverterBase { + private readonly IDataTypeService _dataTypeService; + + //TODO: Remove this ctor in v8 since the other one will use IoC + public MultiNodeTreePickerPropertyConverter() + : this(ApplicationContext.Current.Services.DataTypeService) + { } + + public MultiNodeTreePickerPropertyConverter(IDataTypeService dataTypeService) + : base() + { + if (dataTypeService == null) throw new ArgumentNullException("dataTypeService"); + _dataTypeService = dataTypeService; + } + /// /// The properties to exclude. /// @@ -125,6 +141,24 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters var umbHelper = new UmbracoHelper(UmbracoContext.Current); + Func contentFetcher; + + switch (GetPublishedContentType(propertyType.DataTypeId)) + { + case PublishedItemType.Media: + contentFetcher = umbHelper.TypedMember; + break; + + case PublishedItemType.Member: + contentFetcher = umbHelper.TypedMedia; + break; + + case PublishedItemType.Content: + default: + contentFetcher = umbHelper.TypedContent; + break; + } + if (propertyType.PropertyEditorAlias.Equals(Constants.PropertyEditors.MultiNodeTreePickerAlias)) { var nodeIds = (int[])source; @@ -133,14 +167,9 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters { var multiNodeTreePicker = new List(); - var objectType = UmbracoObjectTypes.Unknown; - foreach (var nodeId in nodeIds) { - var multiNodeTreePickerItem = - GetPublishedContent(nodeId, ref objectType, UmbracoObjectTypes.Document, umbHelper.TypedContent) - ?? GetPublishedContent(nodeId, ref objectType, UmbracoObjectTypes.Media, umbHelper.TypedMedia) - ?? GetPublishedContent(nodeId, ref objectType, UmbracoObjectTypes.Member, umbHelper.TypedMember); + var multiNodeTreePickerItem = contentFetcher(nodeId); if (multiNodeTreePickerItem != null) { @@ -163,14 +192,9 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters { var multiNodeTreePicker = new List(); - var objectType = UmbracoObjectTypes.Unknown; - foreach (var udi in udis) { - var multiNodeTreePickerItem = - GetPublishedContent(udi, ref objectType, UmbracoObjectTypes.Document, umbHelper.TypedContent) - ?? GetPublishedContent(udi, ref objectType, UmbracoObjectTypes.Media, umbHelper.TypedMedia) - ?? GetPublishedContent(udi, ref objectType, UmbracoObjectTypes.Member, umbHelper.TypedMember); + var multiNodeTreePickerItem = contentFetcher(udi); if (multiNodeTreePickerItem != null) { multiNodeTreePicker.Add(multiNodeTreePickerItem); @@ -186,31 +210,50 @@ namespace Umbraco.Web.PropertyEditors.ValueConverters return source; } - /// - /// Attempt to get an IPublishedContent instance based on ID and content type - /// - /// The content node ID - /// The type of content being requested - /// The type of content expected/supported by - /// A function to fetch content of type - /// The requested content, or null if either it does not exist or does not match - private IPublishedContent GetPublishedContent(T nodeId, ref UmbracoObjectTypes actualType, UmbracoObjectTypes expectedType, Func contentFetcher) + private PublishedItemType GetPublishedContentType(int dataTypeId) { - // is the actual type supported by the content fetcher? - if (actualType != UmbracoObjectTypes.Unknown && actualType != expectedType) - { - // no, return null - return null; - } + // GetPreValuesCollectionByDataTypeId is cached at repository level; + // still, the collection is deep-cloned so this is kinda expensive, + // better to cache here + trigger refresh in DataTypeCacheRefresher + // e.g. https://github.com/umbraco/Umbraco-CMS/blob/dev-v7/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs#L116-L119 - // attempt to get the content - var content = contentFetcher(nodeId); - if (content != null) + return Storages.GetOrAdd(dataTypeId, id => { - // if we found the content, assign the expected type to the actual type so we don't have to keep looking for other types of content - actualType = expectedType; - } - return content; + var preValue = _dataTypeService + .GetPreValuesCollectionByDataTypeId(id) + .PreValuesAsDictionary + .FirstOrDefault(x => string.Equals(x.Key, "startNode", StringComparison.InvariantCultureIgnoreCase)) + .Value; + + if (preValue != null && string.IsNullOrWhiteSpace(preValue.Value) == false) + { + var data = JsonConvert.DeserializeAnonymousType(preValue.Value, new { type = string.Empty }); + if (data != null) + { + switch (data.type.ToUpperInvariant()) + { + case "MEDIA": + return PublishedItemType.Media; + + case "MEMBER": + return PublishedItemType.Member; + + case "CONTENT": + default: + return PublishedItemType.Content; + } + } + } + + return PublishedItemType.Content; + }); + } + + private static readonly ConcurrentDictionary Storages = new ConcurrentDictionary(); + + internal static void ClearCaches() + { + Storages.Clear(); } } }