diff --git a/src/Umbraco.Core/XmlHelper.cs b/src/Umbraco.Core/XmlHelper.cs
index 260bb5b86d..5fcca2863e 100644
--- a/src/Umbraco.Core/XmlHelper.cs
+++ b/src/Umbraco.Core/XmlHelper.cs
@@ -128,59 +128,115 @@ namespace Umbraco.Core
}
///
- /// Sorts the children of the parentNode that match the xpath selector
- ///
- ///
- /// An xpath expression used to select child nodes of the XmlElement
- /// An expression that returns true if the XElement passed in is a valid child node to be sorted
- /// The value to order the results by
- internal static void SortNodes(
+ /// Sorts the children of a parentNode.
+ ///
+ /// The parent node.
+ /// An XPath expression to select children of to sort.
+ /// A function returning the value to order the nodes by.
+ internal static void SortNodes(
XmlNode parentNode,
- string childXPathSelector,
- Func childSelector,
- Func orderByValue)
+ string childNodesXPath,
+ Func orderBy)
+ {
+ var sortedChildNodes = parentNode.SelectNodes(childNodesXPath).Cast()
+ .OrderBy(orderBy)
+ .ToArray();
+
+ // append child nodes to last position, in sort-order
+ // so all child nodes will go after the property nodes
+ foreach (var node in sortedChildNodes)
+ parentNode.AppendChild(node); // moves the node to the last position
+ }
+
+ ///
+ /// Sorts the children of a parentNode if needed.
+ ///
+ /// The parent node.
+ /// An XPath expression to select children of to sort.
+ /// A function returning the value to order the nodes by.
+ /// A value indicating whether sorting was needed.
+ /// same as SortNodes but will do nothing if nodes are already sorted - should improve performances.
+ internal static bool SortNodesIfNeeded(
+ XmlNode parentNode,
+ string childNodesXPath,
+ Func orderBy)
+ {
+ // ensure orderBy runs only once per node
+ // checks whether nodes are already ordered
+ // and actually sorts only if needed
+
+ var childNodesAndOrder = parentNode.SelectNodes(childNodesXPath).Cast()
+ .Select(x => Tuple.Create(x, orderBy(x))).ToArray();
+
+ var a = 0;
+ foreach (var x in childNodesAndOrder)
+ {
+ if (a > x.Item2)
+ {
+ a = -1;
+ break;
+ }
+ a = x.Item2;
+ }
+
+ if (a >= 0)
+ return false;
+
+ // append child nodes to last position, in sort-order
+ // so all child nodes will go after the property nodes
+ foreach (var x in childNodesAndOrder.OrderBy(x => x.Item2))
+ parentNode.AppendChild(x.Item1); // moves the node to the last position
+
+ return true;
+ }
+
+ ///
+ /// Sorts a single child node of a parentNode.
+ ///
+ /// The parent node.
+ /// An XPath expression to select children of to sort.
+ /// The child node to sort.
+ /// A function returning the value to order the nodes by.
+ /// A value indicating whether sorting was needed.
+ /// Assuming all nodes but are sorted, this will move the node to
+ /// the right position without moving all the nodes (as SortNodes would do) - should improve perfs.
+ internal static bool SortNode(
+ XmlNode parentNode,
+ string childNodesXPath,
+ XmlNode node,
+ Func orderBy)
{
+ var nodeSortOrder = orderBy(node);
+ var childNodesAndOrder = parentNode.SelectNodes(childNodesXPath).Cast()
+ .Select(x => Tuple.Create(x, orderBy(x))).ToArray();
- var xElement = parentNode.ToXElement();
- var children = xElement.Elements().Where(x => childSelector(x)).ToArray(); //(DONT conver to method group, the build server doesn't like it)
-
- var data = children
- .OrderByDescending(orderByValue) //order by the sort order desc
- .Select(x => children.IndexOf(x)) //store the current item's index (DONT conver to method group, the build server doesn't like it)
- .ToList();
+ // find the first node with a sortOrder > node.sortOrder
+ var i = 0;
+ while (i < childNodesAndOrder.Length && childNodesAndOrder[i].Item2 <= nodeSortOrder)
+ i++;
- //get the minimum index that a content node exists in the parent
- var minElementIndex = xElement.Elements()
- .TakeWhile(x => childSelector(x) == false)
- .Count();
-
- //if the minimum index is zero, then it is the very first node inside the parent,
- // if it is not, we need to store the child property node that exists just before the
- // first content node found so we can insert elements after it when we're sorting.
- var insertAfter = minElementIndex == 0 ? null : parentNode.ChildNodes[minElementIndex - 1];
-
- var selectedChildren = parentNode.SelectNodes(childXPathSelector);
- if (selectedChildren == null)
+ // if one was found
+ if (i < childNodesAndOrder.Length)
{
- throw new InvalidOperationException(string.Format("The childXPathSelector value did not return any results {0}", childXPathSelector));
- }
-
- var childElements = selectedChildren.Cast().ToArray();
-
- //iterate over the ndoes starting with the node with the highest sort order.
- //then we insert this node at the begginning of the parent so that by the end
- //of the iteration the node with the least sort order will be at the top.
- foreach (var node in data.Select(index => childElements[index]))
- {
- if (insertAfter == null)
+ // and node is just before, we're done already
+ // else we need to move it right before the node that was found
+ if (i > 0 && childNodesAndOrder[i - 1].Item1 != node)
{
- parentNode.PrependChild(node);
- }
- else
- {
- parentNode.InsertAfter(node, insertAfter);
+ parentNode.InsertBefore(node, childNodesAndOrder[i].Item1);
+ return true;
}
}
+ else
+ {
+ // and node is the last one, we're done already
+ // else we need to append it as the last one
+ if (i > 0 && childNodesAndOrder[i - 1].Item1 != node)
+ {
+ parentNode.AppendChild(node);
+ return true;
+ }
+ }
+ return false;
}
// used by DynamicNode only, see note in TryCreateXPathDocumentFromPropertyValue
diff --git a/src/Umbraco.Tests/XmlHelperTests.cs b/src/Umbraco.Tests/XmlHelperTests.cs
index 0b7c8fd76b..8050130b51 100644
--- a/src/Umbraco.Tests/XmlHelperTests.cs
+++ b/src/Umbraco.Tests/XmlHelperTests.cs
@@ -86,8 +86,7 @@ namespace Umbraco.Tests
XmlHelper.SortNodes(
parentNode,
"./* [@id]",
- element => element.Attribute("id") != null,
- element => element.AttributeValue("sortOrder"));
+ x => x.AttributeValue("sortOrder"));
watch.Stop();
totalTime += watch.ElapsedMilliseconds;
watch.Reset();
@@ -125,8 +124,7 @@ namespace Umbraco.Tests
XmlHelper.SortNodes(
parentNode,
"./* [@id]",
- element => element.Attribute("id") != null,
- element => element.AttributeValue("sortOrder"));
+ x => x.AttributeValue("sortOrder"));
//do assertions just to make sure it is working properly.
var currSort = 0;
diff --git a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs
index 761ae53e7c..cce6f3f878 100644
--- a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs
+++ b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs
@@ -1,5 +1,6 @@
using System;
using System.Web.Script.Serialization;
+using umbraco;
using Umbraco.Core.Cache;
using Umbraco.Core.Models;
using System.Linq;
@@ -81,6 +82,7 @@ namespace Umbraco.Web.Cache
public override void Refresh(int id)
{
RuntimeCacheProvider.Current.Delete(typeof(IContent), id);
+ content.Instance.UpdateSortOrder(id);
base.Refresh(id);
}
@@ -94,6 +96,7 @@ namespace Umbraco.Web.Cache
public override void Refresh(IContent instance)
{
RuntimeCacheProvider.Current.Delete(typeof(IContent), instance.Id);
+ content.Instance.UpdateSortOrder(instance);
base.Refresh(instance);
}
@@ -112,6 +115,7 @@ namespace Umbraco.Web.Cache
foreach (var payload in DeserializeFromJsonPayload(jsonPayload))
{
RuntimeCacheProvider.Current.Delete(typeof(IContent), payload.Id);
+ content.Instance.UpdateSortOrder(payload.Id);
}
OnCacheUpdated(Instance, new CacheRefresherEventArgs(jsonPayload, MessageType.RefreshByJson));
diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs
index b99ebf4384..65de07994d 100644
--- a/src/Umbraco.Web/umbraco.presentation/content.cs
+++ b/src/Umbraco.Web/umbraco.presentation/content.cs
@@ -2,12 +2,14 @@ using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
+using System.Globalization;
using System.IO;
using System.Linq;
using System.Threading;
using System.Web;
using System.Xml;
using System.Xml.XPath;
+using umbraco.cms.presentation;
using Umbraco.Core;
using Umbraco.Core.Cache;
using Umbraco.Core.Configuration;
@@ -19,12 +21,14 @@ using umbraco.BusinessLogic.Utils;
using umbraco.cms.businesslogic;
using umbraco.cms.businesslogic.cache;
using umbraco.cms.businesslogic.web;
+using Umbraco.Core.Models;
using umbraco.DataLayer;
using umbraco.presentation.nodeFactory;
using Umbraco.Web;
using Action = umbraco.BusinessLogic.Actions.Action;
using Node = umbraco.NodeFactory.Node;
using Umbraco.Core;
+using File = System.IO.File;
namespace umbraco
{
@@ -322,7 +326,7 @@ namespace umbraco
}
}
- public static void TransferValuesFromDocumentXmlToPublishedXml(XmlNode DocumentNode, XmlNode PublishedNode)
+ private static void TransferValuesFromDocumentXmlToPublishedXml(XmlNode DocumentNode, XmlNode PublishedNode)
{
// Remove all attributes and data nodes from the published node
PublishedNode.Attributes.RemoveAll();
@@ -353,8 +357,12 @@ namespace umbraco
if (d.Published)
{
var parentId = d.Level == 1 ? -1 : d.Parent.Id;
- xmlContentCopy = AppendDocumentXml(d.Id, d.Level, parentId,
- GetPreviewOrPublishedNode(d, xmlContentCopy, false), xmlContentCopy);
+
+ // fix sortOrder - see note in UpdateSortOrder
+ var node = GetPreviewOrPublishedNode(d, xmlContentCopy, false);
+ var attr = ((XmlElement)node).GetAttributeNode("sortOrder");
+ attr.Value = d.sortOrder.ToString();
+ xmlContentCopy = AppendDocumentXml(d.Id, d.Level, parentId, node, xmlContentCopy);
// update sitemapprovider
if (updateSitemapProvider && SiteMap.Provider is UmbracoSiteMapProvider)
@@ -382,79 +390,113 @@ namespace umbraco
return xmlContentCopy;
}
- public static XmlDocument AppendDocumentXml(int id, int level, int parentId, XmlNode docNode, XmlDocument xmlContentCopy)
+ // appends a node (docNode) into a cache (xmlContentCopy)
+ // and returns a cache (not necessarily the original one)
+ //
+ internal static XmlDocument AppendDocumentXml(int id, int level, int parentId, XmlNode docNode, XmlDocument xmlContentCopy)
{
- // Find the document in the xml cache
+ // sanity checks
+ if (id != docNode.AttributeValue("id"))
+ throw new ArgumentException("Values of id and docNode/@id are different.");
+ if (parentId != docNode.AttributeValue("parentID"))
+ throw new ArgumentException("Values of parentId and docNode/@parentID are different.");
+
+ // find the document in the cache
XmlNode currentNode = xmlContentCopy.GetElementById(id.ToString());
// if the document is not there already then it's a new document
// we must make sure that its document type exists in the schema
- var xmlContentCopy2 = xmlContentCopy;
if (currentNode == null && UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema == false)
{
- xmlContentCopy = ValidateSchema(docNode.Name, xmlContentCopy);
+ // ValidateSchema looks for the doctype in the schema and if not found
+ // creates a new XML document with a schema containing the doctype. If
+ // a new cache copy is returned, must import the new node into the new
+ // copy.
+ var xmlContentCopy2 = xmlContentCopy;
+ xmlContentCopy = ValidateSchema(docNode.Name, xmlContentCopy);
if (xmlContentCopy != xmlContentCopy2)
docNode = xmlContentCopy.ImportNode(docNode, true);
}
- // Find the parent (used for sortering and maybe creation of new node)
+ // find the parent
XmlNode parentNode = level == 1
- ? xmlContentCopy.DocumentElement
- : xmlContentCopy.GetElementById(parentId.ToString());
+ ? xmlContentCopy.DocumentElement
+ : xmlContentCopy.GetElementById(parentId.ToString());
- if (parentNode != null)
+ // no parent = cannot do anything
+ if (parentNode == null)
+ return xmlContentCopy;
+
+ // define xpath for getting the children nodes (not properties) of a node
+ var childNodesXPath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema
+ ? "./node"
+ : "./* [@id]";
+
+ // insert/move the node under the parent
+ if (currentNode == null)
{
- if (currentNode == null)
+ // document not there, new node, append
+ currentNode = docNode;
+ parentNode.AppendChild(currentNode);
+ }
+ else
+ {
+ // document found... we could just copy the currentNode children nodes over under
+ // docNode, then remove currentNode and insert docNode... the code below tries to
+ // be clever and faster, though only benchmarking could tell whether it's worth the
+ // pain...
+
+ // first copy current parent ID - so we can compare with target parent
+ var moving = currentNode.AttributeValue("parentID") != parentId;
+
+ if (docNode.Name == currentNode.Name)
{
- currentNode = docNode;
- parentNode.AppendChild(currentNode);
+ // name has not changed, safe to just update the current node
+ // by transfering values eg copying the attributes, and importing the data elements
+ TransferValuesFromDocumentXmlToPublishedXml(docNode, currentNode);
+
+ // if moving, move the node to the new parent
+ // else it's already under the right parent
+ // (but maybe the sort order has been updated)
+ if (moving)
+ parentNode.AppendChild(currentNode); // remove then append to parentNode
}
else
{
- //check the current parent id
- var currParentId = currentNode.AttributeValue("parentID");
+ // name has changed, must use docNode (with new name)
+ // move children nodes from currentNode to docNode (already has properties)
+ foreach (XmlNode child in currentNode.SelectNodes(childNodesXPath))
+ docNode.AppendChild(child); // remove then append to docNode
- //update the node with it's new values
- TransferValuesFromDocumentXmlToPublishedXml(docNode, currentNode);
-
- //If the node is being moved we also need to ensure that it exists under the new parent!
- // http://issues.umbraco.org/issue/U4-2312
- // we were never checking this before and instead simply changing the parentId value but not
- // changing the actual parent.
-
- //check the new parent
- if (currParentId != currentNode.AttributeValue("parentID"))
+ // and put docNode in the right place - if parent has not changed, then
+ // just replace, else remove currentNode and insert docNode under the right parent
+ // (but maybe not at the right position due to sort order)
+ if (moving)
{
- //ok, we've actually got to move the node
- parentNode.AppendChild(currentNode);
+ currentNode.ParentNode.RemoveChild(currentNode);
+ parentNode.AppendChild(docNode);
}
-
- }
-
- // TODO: Update with new schema!
- var xpath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema
- ? "./node"
- : "./* [@id]";
-
- var childNodes = parentNode.SelectNodes(xpath);
-
- // Sort the nodes if the added node has a lower sortorder than the last
- if (childNodes != null && childNodes.Count > 0)
- {
- //get the biggest sort order for all children including the one added
- var largestSortOrder = childNodes.Cast().Max(x => x.AttributeValue("sortOrder"));
- var currentSortOrder = currentNode.AttributeValue("sortOrder");
- //if the current item's sort order is less than the largest sort order in the list then
- //we need to resort the xml structure since this item belongs somewhere in the middle.
- //http://issues.umbraco.org/issue/U4-509
- if (childNodes.Count > 1 && currentSortOrder < largestSortOrder)
+ else
{
- SortNodes(ref parentNode);
+ // replacing might screw the sort order
+ parentNode.ReplaceChild(docNode, currentNode);
}
+
+ currentNode = docNode;
}
}
- return xmlContentCopy;
+ // if the nodes are not ordered, must sort
+ // (see U4-509 + has to work with ReplaceChild too)
+ //XmlHelper.SortNodesIfNeeded(parentNode, childNodesXPath, x => x.AttributeValue("sortOrder"));
+
+ // but...
+ // if we assume that nodes are always correctly sorted
+ // then we just need to ensure that currentNode is at the right position.
+ // should be faster that moving all the nodes around.
+ XmlHelper.SortNode(parentNode, childNodesXPath, currentNode, x => x.AttributeValue("sortOrder"));
+
+ return xmlContentCopy;
}
private static XmlNode GetPreviewOrPublishedNode(Document d, XmlDocument xmlContentCopy, bool isPreview)
@@ -472,18 +514,36 @@ namespace umbraco
///
/// Sorts the documents.
///
- /// The parent node.
- public static void SortNodes(ref XmlNode parentNode)
+ /// The parent node identifier.
+ public void SortNodes(int parentId)
{
- var xpath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema
- ? "./node"
- : "./* [@id]";
+ var childNodesXPath = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema
+ ? "./node"
+ : "./* [@id]";
- XmlHelper.SortNodes(
- parentNode,
- xpath,
- element => element.Attribute("id") != null,
- element => element.AttributeValue("sortOrder"));
+ lock (XmlContentInternalSyncLock)
+ {
+ // modify a clone of the cache because even though we're into the write-lock
+ // we may have threads reading at the same time. why is this an option?
+ var wip = UmbracoConfig.For.UmbracoSettings().Content.CloneXmlContent
+ ? CloneXmlDoc(XmlContentInternal)
+ : XmlContentInternal;
+
+ var parentNode = parentId == -1
+ ? XmlContent.DocumentElement
+ : XmlContent.GetElementById(parentId.ToString(CultureInfo.InvariantCulture));
+
+ if (parentNode == null) return;
+
+ var sorted = XmlHelper.SortNodesIfNeeded(
+ parentNode,
+ childNodesXPath,
+ x => x.AttributeValue("sortOrder"));
+
+ if (sorted == false) return;
+
+ XmlContentInternal = wip;
+ }
}
@@ -531,6 +591,50 @@ namespace umbraco
}
}
+ internal virtual void UpdateSortOrder(int contentId)
+ {
+ var content = ApplicationContext.Current.Services.ContentService.GetById(contentId);
+ if (content != null) return;
+ UpdateSortOrder(content);
+ }
+
+ internal virtual void UpdateSortOrder(IContent c)
+ {
+ // the XML in database is updated only when content is published, and then
+ // it contains the sortOrder value at the time the XML was generated. when
+ // a document with unpublished changes is sorted, then it is simply saved
+ // (see ContentService) and so the sortOrder has changed but the XML has
+ // not been updated accordingly.
+
+ // this updates the published cache to take care of the situation
+ // without ContentService having to ... what exactly?
+
+ // no need to do it if the content is published without unpublished changes,
+ // though, because in that case the XML will get re-generated with the
+ // correct sort order.
+ if (c.Published)
+ return;
+
+ lock (XmlContentInternalSyncLock)
+ {
+ var wip = UmbracoConfig.For.UmbracoSettings().Content.CloneXmlContent
+ ? CloneXmlDoc(XmlContentInternal)
+ : XmlContentInternal;
+
+ var node = wip.GetElementById(c.Id.ToString());
+ if (node == null) return;
+ var attr = node.GetAttributeNode("sortOrder");
+ var sortOrder = c.SortOrder.ToString();
+ if (attr.Value == sortOrder) return;
+
+ // only if node was actually modified
+ attr.Value = sortOrder;
+ XmlContentInternal = wip;
+
+ // no need to clear any cache
+ }
+ }
+
///
/// Updates the document cache for multiple documents
///
@@ -1010,6 +1114,13 @@ order by umbracoNode.level, umbracoNode.sortOrder";
int parentId = dr.GetInt("parentId");
string xml = dr.GetString("xml");
+ // fix sortOrder - see notes in UpdateSortOrder
+ var tmp = new XmlDocument();
+ tmp.LoadXml(xml);
+ var attr = tmp.DocumentElement.GetAttributeNode("sortOrder");
+ attr.Value = dr.GetInt("sortOrder").ToString();
+ xml = tmp.InnerXml;
+
// Call the eventhandler to allow modification of the string
var e1 = new ContentCacheLoadNodeEventArgs();
FireAfterContentCacheDatabaseLoadXmlString(ref xml, e1);
diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/nodeSorter.asmx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/nodeSorter.asmx.cs
index 1dfa3362da..2177fedeb1 100644
--- a/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/nodeSorter.asmx.cs
+++ b/src/Umbraco.Web/umbraco.presentation/umbraco/webservices/nodeSorter.asmx.cs
@@ -155,14 +155,9 @@ namespace umbraco.presentation.webservices
// Save content with new sort order and update db+cache accordingly
var sorted = contentService.Sort(sortedContent);
- // Refresh sort order on cached xml
- XmlNode parentNode = parentId == -1
- ? content.Instance.XmlContent.DocumentElement
- : content.Instance.XmlContent.GetElementById(parentId.ToString(CultureInfo.InvariantCulture));
-
- //only try to do the content sort if the the parent node is available...
- if (parentNode != null)
- content.SortNodes(ref parentNode);
+ // refresh sort order on cached xml
+ // but no... this is not distributed - solely relying on content service & events should be enough
+ //content.Instance.SortNodes(parentId);
//send notifications! TODO: This should be put somewhere centralized instead of hard coded directly here
ApplicationContext.Services.NotificationService.SendNotification(contentService.GetById(parentId), ActionSort.Instance, UmbracoContext, ApplicationContext);