From a8f6b9eeb9ceebf6c2c85cbee4f526626beda8f6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 30 Sep 2013 13:31:48 +1000 Subject: [PATCH] Fixes: U4-2925 Tree performance is very poor in 6.1.x when having event subscriptions --- src/Umbraco.Core/TypeHelper.cs | 10 ++ .../Trees/BaseContentTreeTests.cs | 156 +++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + .../umbraco/Trees/BaseContentTree.cs | 129 +++++++++----- .../umbraco/Trees/BaseMediaTree.cs | 163 ++++++++++++------ .../umbraco/Trees/BaseTree.cs | 10 ++ 6 files changed, 367 insertions(+), 102 deletions(-) create mode 100644 src/Umbraco.Tests/Trees/BaseContentTreeTests.cs diff --git a/src/Umbraco.Core/TypeHelper.cs b/src/Umbraco.Core/TypeHelper.cs index e77f03ee78..adda1d3298 100644 --- a/src/Umbraco.Core/TypeHelper.cs +++ b/src/Umbraco.Core/TypeHelper.cs @@ -16,6 +16,16 @@ namespace Umbraco.Core private static readonly ConcurrentDictionary GetFieldsCache = new ConcurrentDictionary(); private static readonly ConcurrentDictionary, PropertyInfo[]> GetPropertiesCache = new ConcurrentDictionary, PropertyInfo[]>(); + /// + /// Checks if the method is actually overriding a base method + /// + /// + /// + public static bool IsOverride(MethodInfo m) + { + return m.GetBaseDefinition().DeclaringType != m.DeclaringType; + } + /// /// Find all assembly references that are referencing the assignTypeFrom Type's assembly found in the assemblyList /// diff --git a/src/Umbraco.Tests/Trees/BaseContentTreeTests.cs b/src/Umbraco.Tests/Trees/BaseContentTreeTests.cs new file mode 100644 index 0000000000..70c4ed9326 --- /dev/null +++ b/src/Umbraco.Tests/Trees/BaseContentTreeTests.cs @@ -0,0 +1,156 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using NUnit.Framework; +using umbraco.cms.presentation.Trees; + +namespace Umbraco.Tests.Trees +{ + + [TestFixture] + public class BaseMediaTreeTests + { + + [TearDown] + public void TestTearDown() + { + BaseTree.AfterTreeRender -= EventHandler; + BaseTree.BeforeTreeRender -= EventHandler; + } + + [Test] + public void Run_Optimized() + { + var tree = new MyOptimizedMediaTree("media"); + + Assert.IsTrue(tree.UseOptimizedRendering); + } + + [Test] + public void Not_Optimized_Events_AfterRender() + { + var tree = new MyOptimizedMediaTree("media"); + + BaseTree.AfterTreeRender += EventHandler; + + Assert.IsFalse(tree.UseOptimizedRendering); + } + + [Test] + public void Not_Optimized_Events_BeforeRender() + { + var tree = new MyOptimizedMediaTree("media"); + + BaseTree.BeforeTreeRender += EventHandler; + + Assert.IsFalse(tree.UseOptimizedRendering); + } + + private void EventHandler(object sender, TreeEventArgs treeEventArgs) + { + + } + + public class MyOptimizedMediaTree : BaseMediaTree + { + public MyOptimizedMediaTree(string application) + : base(application) + { + } + + protected override void CreateRootNode(ref XmlTreeNode rootNode) + { + + } + } + + + } + + [TestFixture] + public class BaseContentTreeTests + { + + [TearDown] + public void TestTearDown() + { + BaseTree.AfterTreeRender -= EventHandler; + BaseTree.BeforeTreeRender -= EventHandler; + } + + [Test] + public void Run_Optimized() + { + var tree = new MyOptimizedContentTree("content"); + + Assert.IsTrue(tree.UseOptimizedRendering); + } + + [Test] + public void Not_Optimized_Events_AfterRender() + { + var tree = new MyOptimizedContentTree("content"); + + BaseTree.AfterTreeRender += EventHandler; + + Assert.IsFalse(tree.UseOptimizedRendering); + } + + [Test] + public void Not_Optimized_Events_BeforeRender() + { + var tree = new MyOptimizedContentTree("content"); + + BaseTree.BeforeTreeRender += EventHandler; + + Assert.IsFalse(tree.UseOptimizedRendering); + } + + [Test] + public void Not_Optimized_Overriden_Method() + { + var tree = new MyNotOptimizedContentTree("content"); + + Assert.IsFalse(tree.UseOptimizedRendering); + } + + private void EventHandler(object sender, TreeEventArgs treeEventArgs) + { + + } + + public class MyOptimizedContentTree : BaseContentTree + { + public MyOptimizedContentTree(string application) + : base(application) + { + } + + protected override void CreateRootNode(ref XmlTreeNode rootNode) + { + + } + } + + public class MyNotOptimizedContentTree : BaseContentTree + { + public MyNotOptimizedContentTree(string application) + : base(application) + { + } + + protected override void CreateRootNode(ref XmlTreeNode rootNode) + { + + } + + protected override void OnRenderNode(ref XmlTreeNode xNode, umbraco.cms.businesslogic.web.Document doc) + { + base.OnRenderNode(ref xNode, doc); + } + } + + + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 34c578d136..003b7611a3 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -362,6 +362,7 @@ + diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs index 39eb33e31f..bb1046985b 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs @@ -2,8 +2,10 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Reflection; using System.Text; using System.Web; +using Umbraco.Core; using Umbraco.Core.Models; using umbraco.BasePages; using umbraco.BusinessLogic; @@ -73,61 +75,64 @@ function openContent(id) { /// Renders the specified tree item. /// /// The tree. - /*public override void Render(ref XmlTree Tree) - { - //get documents to render - Document[] docs = Document.GetChildrenForTree(m_id); - - var args = new TreeEventArgs(Tree); - OnBeforeTreeRender(docs, args); - - foreach (Document dd in docs) - { - List allowedUserOptions = GetUserActionsForNode(dd); - if (CanUserAccessNode(dd, allowedUserOptions)) - { - - XmlTreeNode node = CreateNode(dd, allowedUserOptions); - - OnRenderNode(ref node, dd); - - OnBeforeNodeRender(ref Tree, ref node, EventArgs.Empty); - if (node != null) - { - Tree.Add(node); - OnAfterNodeRender(ref Tree, ref node, EventArgs.Empty); - } - } - } - OnAfterTreeRender(docs, args); - }*/ public override void Render(ref XmlTree Tree) { - //get documents to render - var entities = Services.EntityService.GetChildren(m_id, UmbracoObjectTypes.Document).ToArray(); - - var args = new TreeEventArgs(Tree); - OnBeforeTreeRender(entities, args, true); - - foreach (var entity in entities) + if (UseOptimizedRendering == false) { - var e = entity as UmbracoEntity; - List allowedUserOptions = GetUserActionsForNode(e); - if (CanUserAccessNode(e, allowedUserOptions)) + //We cannot run optimized mode since there are subscribers to events/methods that require document instances + // so we'll render the original way by looking up the docs. + + //get documents to render + var docs = Document.GetChildrenForTree(m_id); + + var args = new TreeEventArgs(Tree); + OnBeforeTreeRender(docs, args); + + foreach (var dd in docs) { - XmlTreeNode node = CreateNode(e, allowedUserOptions); - - OnRenderNode(ref node, new Document(entity, LoadMinimalDocument)); - - OnBeforeNodeRender(ref Tree, ref node, EventArgs.Empty); - if (node != null) + var allowedUserOptions = GetUserActionsForNode(dd); + if (CanUserAccessNode(dd, allowedUserOptions)) { - Tree.Add(node); - OnAfterNodeRender(ref Tree, ref node, EventArgs.Empty); + + var node = CreateNode(dd, allowedUserOptions); + + OnRenderNode(ref node, dd); + + OnBeforeNodeRender(ref Tree, ref node, EventArgs.Empty); + if (node != null) + { + Tree.Add(node); + OnAfterNodeRender(ref Tree, ref node, EventArgs.Empty); + } + } + } + OnAfterTreeRender(docs, args); + } + else + { + + //We ARE running in optmized mode, this means we will NOT be raising the BeforeTreeRender or AfterTreeRender + // events and NOT calling the OnRenderNode method - we've already detected that there are not subscribers or implementations + // to call so that is fine. + + var entities = Services.EntityService.GetChildren(m_id, UmbracoObjectTypes.Document).ToArray(); + foreach (var entity in entities) + { + var e = entity as UmbracoEntity; + var allowedUserOptions = GetUserActionsForNode(e); + if (CanUserAccessNode(e, allowedUserOptions)) + { + var node = CreateNode(e, allowedUserOptions); + + OnBeforeNodeRender(ref Tree, ref node, EventArgs.Empty); + if (node != null) + { + Tree.Add(node); + OnAfterNodeRender(ref Tree, ref node, EventArgs.Empty); + } } } } - OnAfterTreeRender(entities, args, true); } #region Tree Create-node-helper Methods - Legacy @@ -475,5 +480,35 @@ function openContent(id) { return umbraco.BusinessLogic.Actions.Action.FromString(fullMenu); } + /// + /// Returns true if we can use the EntityService to render the tree or revert to the original way + /// using normal documents + /// + /// + /// We determine this by: + /// * If there are any subscribers to the events: BeforeTreeRender or AfterTreeRender - then we cannot run optimized + /// * If there are any overrides of the method: OnRenderNode - then we cannot run optimized + /// + internal bool UseOptimizedRendering + { + get + { + if (HasEntityBasedEventSubscribers) + { + return false; + } + + //now we need to check if the current tree type has OnRenderNode overridden with a custom implementation + //Strangely - this even works in med trust! + var method = this.GetType().GetMethod("OnRenderNode", BindingFlags.Instance | BindingFlags.NonPublic, null, new[] { typeof(XmlTreeNode).MakeByRefType(), typeof(Document) }, null); + if (TypeHelper.IsOverride(method)) + { + return false; + } + + return true; + } + } + } } diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseMediaTree.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseMediaTree.cs index f4e4684c72..e501489534 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseMediaTree.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseMediaTree.cs @@ -2,12 +2,14 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Reflection; using System.Text; using Umbraco.Core.Logging; using Umbraco.Core.Models; using umbraco.BasePages; using umbraco.BusinessLogic; using umbraco.BusinessLogic.Actions; +using umbraco.cms.businesslogic.web; using umbraco.interfaces; using Umbraco.Core; using Media = umbraco.cms.businesslogic.media.Media; @@ -17,7 +19,6 @@ namespace umbraco.cms.presentation.Trees { public abstract class BaseMediaTree : BaseTree { - private DisposableTimer _timer; private User _user; public BaseMediaTree(string application) @@ -57,78 +58,109 @@ function openMedia(id) { } } - //Updated Render method for improved performance, but currently not usable because of backwards compatibility - //with the OnBeforeTreeRender/OnAfterTreeRender events, which sends an array for legacy Media items. public override void Render(ref XmlTree tree) { - //_timer = DisposableTimer.Start(x => LogHelper.Debug("Media tree loaded" + " (took " + x + "ms)")); - var entities = Services.EntityService.GetChildren(m_id, UmbracoObjectTypes.Media).ToArray(); - - var args = new TreeEventArgs(tree); - OnBeforeTreeRender(entities, args, false); - - foreach (UmbracoEntity entity in entities) + if (UseOptimizedRendering == false) { - XmlTreeNode xNode = XmlTreeNode.Create(this); - xNode.NodeID = entity.Id.ToString(CultureInfo.InvariantCulture); - xNode.Text = entity.Name; + //We cannot run optimized mode since there are subscribers to events/methods that require document instances + // so we'll render the original way by looking up the docs. - xNode.HasChildren = entity.HasChildren; - xNode.Source = this.IsDialog ? GetTreeDialogUrl(entity.Id) : GetTreeServiceUrl(entity.Id); + var docs = new Media(m_id).Children; - xNode.Icon = entity.ContentTypeIcon; - xNode.OpenIcon = entity.ContentTypeIcon; - - if (IsDialog == false) + var args = new TreeEventArgs(tree); + OnBeforeTreeRender(docs, args); + + foreach (var dd in docs) { - if(this.ShowContextMenu == false) - xNode.Menu = null; - xNode.Action = "javascript:openMedia(" + entity.Id + ");"; - } - else - { - xNode.Menu = this.ShowContextMenu ? new List(new IAction[] { ActionRefresh.Instance }) : null; - if (this.DialogMode == TreeDialogModes.fulllink) + var e = dd; + var xNode = PerformNodeRender(e.Id, e.Text, e.HasChildren, e.ContentType.IconUrl, e.ContentType.Alias, () => GetLinkValue(e, e.Id.ToString(CultureInfo.InvariantCulture))); + + + OnBeforeNodeRender(ref tree, ref xNode, EventArgs.Empty); + if (xNode != null) { - string nodeLink = GetLinkValue(entity); - if (string.IsNullOrEmpty(nodeLink) == false) - { - xNode.Action = "javascript:openMedia('" + nodeLink + "');"; - } - else - { - if (string.Equals(entity.ContentTypeAlias, Constants.Conventions.MediaTypes.Folder, StringComparison.OrdinalIgnoreCase)) - { - //#U4-2254 - Inspiration to use void from here: http://stackoverflow.com/questions/4924383/jquery-object-object-error - xNode.Action = "javascript:void jQuery('.umbTree #" + entity.Id.ToString(CultureInfo.InvariantCulture) + "').click();"; - } - else - { - xNode.Action = null; - xNode.Style.DimNode(); - } - } + tree.Add(xNode); + OnAfterNodeRender(ref tree, ref xNode, EventArgs.Empty); + } + } + + OnAfterTreeRender(docs, args); + } + else + { + //We ARE running in optmized mode, this means we will NOT be raising the BeforeTreeRender or AfterTreeRender + // events - we've already detected that there are not subscribers or implementations + // to call so that is fine. + + var entities = Services.EntityService.GetChildren(m_id, UmbracoObjectTypes.Media).ToArray(); + + foreach (UmbracoEntity entity in entities) + { + var e = entity; + var xNode = PerformNodeRender(e.Id, entity.Name, e.HasChildren, e.ContentTypeIcon, e.ContentTypeAlias, () => GetLinkValue(e)); + + OnBeforeNodeRender(ref tree, ref xNode, EventArgs.Empty); + if (xNode != null) + { + tree.Add(xNode); + OnAfterNodeRender(ref tree, ref xNode, EventArgs.Empty); + } + } + } + } + + private XmlTreeNode PerformNodeRender(int nodeId, string nodeName, bool hasChildren, string icon, string contentTypeAlias, Func getLinkValue) + { + var xNode = XmlTreeNode.Create(this); + xNode.NodeID = nodeId.ToString(CultureInfo.InvariantCulture); + xNode.Text = nodeName; + + xNode.HasChildren = hasChildren; + xNode.Source = this.IsDialog ? GetTreeDialogUrl(nodeId) : GetTreeServiceUrl(nodeId); + + xNode.Icon = icon; + xNode.OpenIcon = icon; + + if (IsDialog == false) + { + if (this.ShowContextMenu == false) + xNode.Menu = null; + xNode.Action = "javascript:openMedia(" + nodeId + ");"; + } + else + { + xNode.Menu = this.ShowContextMenu ? new List(new IAction[] { ActionRefresh.Instance }) : null; + if (this.DialogMode == TreeDialogModes.fulllink) + { + string nodeLink = getLinkValue(); + if (string.IsNullOrEmpty(nodeLink) == false) + { + xNode.Action = "javascript:openMedia('" + nodeLink + "');"; } else { - xNode.Action = "javascript:openMedia('" + entity.Id.ToString(CultureInfo.InvariantCulture) + "');"; + if (string.Equals(contentTypeAlias, Constants.Conventions.MediaTypes.Folder, StringComparison.OrdinalIgnoreCase)) + { + //#U4-2254 - Inspiration to use void from here: http://stackoverflow.com/questions/4924383/jquery-object-object-error + xNode.Action = "javascript:void jQuery('.umbTree #" + nodeId.ToString(CultureInfo.InvariantCulture) + "').click();"; + } + else + { + xNode.Action = null; + xNode.Style.DimNode(); + } } } - - OnBeforeNodeRender(ref tree, ref xNode, EventArgs.Empty); - if (xNode != null) + else { - tree.Add(xNode); - OnAfterNodeRender(ref tree, ref xNode, EventArgs.Empty); + xNode.Action = "javascript:openMedia('" + nodeId.ToString(CultureInfo.InvariantCulture) + "');"; } } - //stop the timer and log the output - //_timer.Dispose(); - - OnAfterTreeRender(entities, args, false); + return xNode; } + /// /// Returns the value for a link in WYSIWYG mode, by default only media items that have a @@ -183,5 +215,26 @@ function openMedia(id) { /// public static List LinkableMediaDataTypes { get; protected set; } + /// + /// Returns true if we can use the EntityService to render the tree or revert to the original way + /// using normal documents + /// + /// + /// We determine this by: + /// * If there are any subscribers to the events: BeforeTreeRender or AfterTreeRender - then we cannot run optimized + /// + internal bool UseOptimizedRendering + { + get + { + if (HasEntityBasedEventSubscribers) + { + return false; + } + + return true; + } + } + } } diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseTree.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseTree.cs index 08038f0421..9c547edf04 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseTree.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseTree.cs @@ -527,6 +527,7 @@ namespace umbraco.cms.presentation.Trees AfterTreeRender(sender, e); } + [Obsolete("Do not use this method to raise events, it is no longer used and will cause very high performance spikes!")] protected internal virtual void OnBeforeTreeRender(IEnumerable sender, TreeEventArgs e, bool isContent) { if (BeforeTreeRender != null) @@ -542,6 +543,7 @@ namespace umbraco.cms.presentation.Trees } } + [Obsolete("Do not use this method to raise events, it is no longer used and will cause very high performance spikes!")] protected internal virtual void OnAfterTreeRender(IEnumerable sender, TreeEventArgs e, bool isContent) { if (AfterTreeRender != null) @@ -557,6 +559,14 @@ namespace umbraco.cms.presentation.Trees } } + /// + /// Returns true if there are subscribers to either BeforeTreeRender or AfterTreeRender + /// + internal bool HasEntityBasedEventSubscribers + { + get { return BeforeTreeRender != null || AfterTreeRender != null; } + } + /// /// Event that is raised once actions are assigned to nodes ///