From 11449688baf1c4d83e81903bb779acdad1e2e2ca Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Tue, 5 Feb 2013 04:29:01 +0600 Subject: [PATCH] Fixes: #U4-1636 - We still use DynamicXml but we don't make a call to StripDashesInElementOrAttributeNames before creating the DynamicXml object, instead the DynamicXml object itself will test whether the stripped values match or not. Have updated/created unit tests to confirm this functionality. --- src/Umbraco.Core/Dynamics/DynamicXml.cs | 205 ++++++++++-------- src/Umbraco.Core/PublishedContentHelper.cs | 2 +- src/Umbraco.Core/XmlHelper.cs | 54 +++++ .../DynamicXmlConverterTests.cs | 62 ++++++ .../PublishedContent/DynamicXmlTests.cs | 91 +++----- src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 6 files changed, 269 insertions(+), 146 deletions(-) create mode 100644 src/Umbraco.Tests/PublishedContent/DynamicXmlConverterTests.cs diff --git a/src/Umbraco.Core/Dynamics/DynamicXml.cs b/src/Umbraco.Core/Dynamics/DynamicXml.cs index f0942d02a8..a099099970 100644 --- a/src/Umbraco.Core/Dynamics/DynamicXml.cs +++ b/src/Umbraco.Core/Dynamics/DynamicXml.cs @@ -16,6 +16,23 @@ namespace Umbraco.Core.Dynamics public class DynamicXml : DynamicObject, IEnumerable, IEnumerable { public XElement BaseElement { get; set; } + private XElement _cleanedBaseElement; + + /// + /// Returns a cleaned Xml element which is purely used for matching against names for elements and attributes + /// when the normal names don't match, for example when the original names contain hyphens. + /// + /// + private XElement GetCleanedBaseElement() + { + if (_cleanedBaseElement == null) + { + if (BaseElement == null) + throw new InvalidOperationException("Cannot return a cleaned XML element when the BaseElement is null"); + _cleanedBaseElement = XElement.Parse(XmlHelper.StripDashesInElementOrAttributeNames(BaseElement.ToString())); + } + return _cleanedBaseElement; + } public DynamicXml(XElement baseElement) { @@ -128,50 +145,112 @@ namespace Umbraco.Core.Dynamics result = null; return false; } - //Go ahead and try to fetch all of the elements matching the member name, and wrap them - var elements = BaseElement.Elements(binder.Name); - if (!elements.Any() && BaseElement.Name == "root" && BaseElement.Elements().Count() == 1) - { - //no elements matched, lets try first child - elements = BaseElement.Elements().ElementAt(0).Elements(binder.Name); - } - if (HandleIEnumerableXElement(elements, out result)) - { - return true; - } - else - { - //Ok, so no elements matched, so lets try attributes - IEnumerable attributes = BaseElement.Attributes(binder.Name).Select(attr => attr.Value); - int count = attributes.Count(); - - if (count > 0) + //First check for matching name including the 'cleaned' name (i.e. removal of hyphens, etc... ) + var elementByNameAttempt = CheckNodeNameMatch(binder.Name, BaseElement, true); + if (elementByNameAttempt.Success) + { + if (HandleIEnumerableXElement(elementByNameAttempt.Result, out result)) { - if (count > 1) - result = attributes; //more than one attribute matched, lets return the collection - else - result = attributes.FirstOrDefault(); //only one attribute matched, lets just return it - - return true; // return true because we matched + return true; + } + } + + //Ok, so no elements matched, so lets try attributes + var attributeByNameAttempt = CheckAttributeNameMatch(binder.Name, BaseElement, true); + if (attributeByNameAttempt.Success) + { + if (attributeByNameAttempt.Result.Count() > 1) + { + //more than one attribute matched, lets return the collection + result = attributeByNameAttempt.Result; } else { - //no attributes matched, lets try first child - if (BaseElement.Name == "root" && BaseElement.Elements().Count() == 1) - { - attributes = BaseElement.Elements().ElementAt(0).Attributes(binder.Name).Select(attr => attr.Value); - count = attributes.Count(); - if (count > 1) - result = attributes; //more than one attribute matched, lets return the collection - else - result = attributes.FirstOrDefault(); //only one attribute matched, lets just return it + //only one attribute matched, lets just return it + result = attributeByNameAttempt.Result.FirstOrDefault(); + } + return true; + } + - return true; // return true because we matched - } + return base.TryGetMember(binder, out result); + } + + private Attempt> CheckAttributeNameMatch(string name, XElement xmlElement, bool checkCleanedName) + { + var attributes = xmlElement.Attributes(name).Select(attr => attr.Value).ToArray(); + if (attributes.Any()) + { + return new Attempt>(true, attributes); + } + + //NOTE: this seems strange we're checking for the term 'root', this is legacy code so we'll keep it i guess + if (!attributes.Any() && xmlElement.Name == "root" && xmlElement.Elements().Count() == 1) + { + //no elements matched and this node is called 'root' and only has one child... lets see if it matches. + var childElements = xmlElement.Elements().ElementAt(0).Attributes(name).Select(attr => attr.Value).ToArray(); + if (childElements.Any()) + { + //we've found a match by the first child of an element called 'root' (strange, but sure) + return new Attempt>(true, childElements); } } - return base.TryGetMember(binder, out result); + + if (checkCleanedName) + { + //still no match, we'll try to match with a 'cleaned' name + var cleanedXml = GetCleanedBaseElement(); + + //pass false in to this as we don't want an infinite loop and clean the already cleaned xml + return CheckAttributeNameMatch(name, cleanedXml, false); + } + + //no deal + return Attempt>.False; + } + + /// + /// Checks if the 'name' matches any elements of xmlElement + /// + /// The name to match + /// The xml element to check against + /// If there are no matches, we'll clean the xml (i.e. remove hyphens, etc..) and then retry + /// + private Attempt> CheckNodeNameMatch(string name, XElement xmlElement, bool checkCleanedName) + { + //Go ahead and try to fetch all of the elements matching the member name, and wrap them + var elements = xmlElement.Elements(name).ToArray(); + + //Check if we've got any matches, if so then return true + if (elements.Any()) + { + return new Attempt>(true, elements); + } + + //NOTE: this seems strange we're checking for the term 'root', this is legacy code so we'll keep it i guess + if (!elements.Any() && xmlElement.Name == "root" && xmlElement.Elements().Count() == 1) + { + //no elements matched and this node is called 'root' and only has one child... lets see if it matches. + var childElements = xmlElement.Elements().ElementAt(0).Elements(name).ToArray(); + if (childElements.Any()) + { + //we've found a match by the first child of an element called 'root' (strange, but sure) + return new Attempt>(true, childElements); + } + } + + if (checkCleanedName) + { + //still no match, we'll try to match with a 'cleaned' name + var cleanedXml = GetCleanedBaseElement(); + + //pass false in to this as we don't want an infinite loop and clean the already cleaned xml + return CheckNodeNameMatch(name, cleanedXml, false); + } + + //no deal + return Attempt>.False; } private bool HandleIEnumerableXElement(IEnumerable elements, out object result) @@ -193,7 +272,7 @@ namespace Umbraco.Core.Dynamics //We have more than one matching element, so let's return the collection //elements is IEnumerable //but we want to be able to re-enter this code - XElement root = new XElement(XName.Get("root")); + var root = new XElement(XName.Get("root")); root.Add(elements); result = new DynamicXml(root); @@ -205,6 +284,7 @@ namespace Umbraco.Core.Dynamics result = null; return false; } + public DynamicXml XPath(string expression) { var matched = this.BaseElement.XPathSelectElements(expression); @@ -689,57 +769,10 @@ namespace Umbraco.Core.Dynamics return test(this) ? new HtmlString(valueIfTrue) : new HtmlString(valueIfFalse); } + [Obsolete("Use XmlHelper.StripDashesInElementOrAttributeNames instead")] public static string StripDashesInElementOrAttributeNames(string xml) { - using (MemoryStream outputms = new MemoryStream()) - { - using (TextWriter outputtw = new StreamWriter(outputms)) - { - using (MemoryStream ms = new MemoryStream()) - { - using (TextWriter tw = new StreamWriter(ms)) - { - tw.Write(xml); - tw.Flush(); - ms.Position = 0; - using (TextReader tr = new StreamReader(ms)) - { - bool IsInsideElement = false, IsInsideQuotes = false; - int ic = 0; - while ((ic = tr.Read()) != -1) - { - if (ic == (int)'<' && !IsInsideQuotes) - { - if (tr.Peek() != (int)'!') - { - IsInsideElement = true; - } - } - if (ic == (int)'>' && !IsInsideQuotes) - { - IsInsideElement = false; - } - if (ic == (int)'"') - { - IsInsideQuotes = !IsInsideQuotes; - } - if (!IsInsideElement || ic != (int)'-' || IsInsideQuotes) - { - outputtw.Write((char)ic); - } - } - - } - } - } - outputtw.Flush(); - outputms.Position = 0; - using (TextReader outputtr = new StreamReader(outputms)) - { - return outputtr.ReadToEnd(); - } - } - } + return XmlHelper.StripDashesInElementOrAttributeNames(xml); } diff --git a/src/Umbraco.Core/PublishedContentHelper.cs b/src/Umbraco.Core/PublishedContentHelper.cs index ec6c35891a..ada557d217 100644 --- a/src/Umbraco.Core/PublishedContentHelper.cs +++ b/src/Umbraco.Core/PublishedContentHelper.cs @@ -137,7 +137,7 @@ namespace Umbraco.Core { try { - var e = XElement.Parse(DynamicXml.StripDashesInElementOrAttributeNames(sResult), LoadOptions.None); + var e = XElement.Parse(sResult, LoadOptions.None); //check that the document element is not one of the disallowed elements //allows RTE to still return as html if it's valid xhtml diff --git a/src/Umbraco.Core/XmlHelper.cs b/src/Umbraco.Core/XmlHelper.cs index 27f4d0e923..3e67f2728e 100644 --- a/src/Umbraco.Core/XmlHelper.cs +++ b/src/Umbraco.Core/XmlHelper.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.IO; using System.Linq; using System.Text.RegularExpressions; using System.Xml; @@ -14,6 +15,59 @@ namespace Umbraco.Core public class XmlHelper { + public static string StripDashesInElementOrAttributeNames(string xml) + { + using (var outputms = new MemoryStream()) + { + using (TextWriter outputtw = new StreamWriter(outputms)) + { + using (var ms = new MemoryStream()) + { + using (var tw = new StreamWriter(ms)) + { + tw.Write(xml); + tw.Flush(); + ms.Position = 0; + using (var tr = new StreamReader(ms)) + { + bool IsInsideElement = false, IsInsideQuotes = false; + int ic = 0; + while ((ic = tr.Read()) != -1) + { + if (ic == (int)'<' && !IsInsideQuotes) + { + if (tr.Peek() != (int)'!') + { + IsInsideElement = true; + } + } + if (ic == (int)'>' && !IsInsideQuotes) + { + IsInsideElement = false; + } + if (ic == (int)'"') + { + IsInsideQuotes = !IsInsideQuotes; + } + if (!IsInsideElement || ic != (int)'-' || IsInsideQuotes) + { + outputtw.Write((char)ic); + } + } + + } + } + } + outputtw.Flush(); + outputms.Position = 0; + using (TextReader outputtr = new StreamReader(outputms)) + { + return outputtr.ReadToEnd(); + } + } + } + } + /// /// Imports a XML node from text. /// diff --git a/src/Umbraco.Tests/PublishedContent/DynamicXmlConverterTests.cs b/src/Umbraco.Tests/PublishedContent/DynamicXmlConverterTests.cs new file mode 100644 index 0000000000..dad9649564 --- /dev/null +++ b/src/Umbraco.Tests/PublishedContent/DynamicXmlConverterTests.cs @@ -0,0 +1,62 @@ +using System.Xml; +using System.Xml.Linq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Dynamics; +using Umbraco.Tests.PartialTrust; + +namespace Umbraco.Tests.PublishedContent +{ + [TestFixture] + public class DynamicXmlConverterTests : AbstractPartialTrustFixture + { + [Test] + public void Convert_To_String() + { + var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; + var dXml = new DynamicXml(xml); + var result = dXml.TryConvertTo(); + Assert.IsTrue(result.Success); + Assert.AreEqual(xml, result.Result); + } + + [Test] + public void Convert_To_XElement() + { + var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; + var dXml = new DynamicXml(xml); + var result = dXml.TryConvertTo(); + Assert.IsTrue(result.Success); + Assert.AreEqual(xml, result.Result.ToString(SaveOptions.DisableFormatting)); + } + + [Test] + public void Convert_To_XmlElement() + { + var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; + var dXml = new DynamicXml(xml); + var result = dXml.TryConvertTo(); + Assert.IsTrue(result.Success); + Assert.AreEqual(xml, result.Result.OuterXml); + } + + [Test] + public void Convert_To_XmlDocument() + { + var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; + var dXml = new DynamicXml(xml); + var result = dXml.TryConvertTo(); + Assert.IsTrue(result.Success); + Assert.AreEqual(xml, result.Result.InnerXml); + } + + public override void TestSetup() + { + + } + + public override void TestTearDown() + { + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/PublishedContent/DynamicXmlTests.cs b/src/Umbraco.Tests/PublishedContent/DynamicXmlTests.cs index 4fc304730a..a566ce5cee 100644 --- a/src/Umbraco.Tests/PublishedContent/DynamicXmlTests.cs +++ b/src/Umbraco.Tests/PublishedContent/DynamicXmlTests.cs @@ -1,74 +1,47 @@ using System; using System.Diagnostics; -using System.Xml; -using System.Xml.Linq; using Microsoft.CSharp.RuntimeBinder; using NUnit.Framework; using Umbraco.Core.Dynamics; using System.Linq; -using Umbraco.Tests.PartialTrust; -using Umbraco.Core; namespace Umbraco.Tests.PublishedContent { - [TestFixture] - public class DynamicXmlConverterTests : AbstractPartialTrustFixture - { - [Test] - public void Convert_To_String() - { - var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; - var dXml = new DynamicXml(xml); - var result = dXml.TryConvertTo(); - Assert.IsTrue(result.Success); - Assert.AreEqual(xml, result.Result); - } - - [Test] - public void Convert_To_XElement() - { - var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; - var dXml = new DynamicXml(xml); - var result = dXml.TryConvertTo(); - Assert.IsTrue(result.Success); - Assert.AreEqual(xml, result.Result.ToString(SaveOptions.DisableFormatting)); - } - - [Test] - public void Convert_To_XmlElement() - { - var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; - var dXml = new DynamicXml(xml); - var result = dXml.TryConvertTo(); - Assert.IsTrue(result.Success); - Assert.AreEqual(xml, result.Result.OuterXml); - } - - [Test] - public void Convert_To_XmlDocument() - { - var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; - var dXml = new DynamicXml(xml); - var result = dXml.TryConvertTo(); - Assert.IsTrue(result.Success); - Assert.AreEqual(xml, result.Result.InnerXml); - } - - public override void TestSetup() - { - - } - - public override void TestTearDown() - { - } - } - - [TestFixture] + [TestFixture] public class DynamicXmlTests { + /// + /// Ensures that when we return the xml structure we get the real structure, not the replaced hyphen structure + /// see: http://issues.umbraco.org/issue/U4-1405#comment=67-5113 + /// http://issues.umbraco.org/issue/U4-1636 + /// + [Test] + public void Deals_With_Hyphenated_values() + { + var xml = @" + + True + 1161 + /content/ + 12 december Zorgbeurs Care + +"; - [Test] + var typedXml = new DynamicXml(xml); + dynamic dynamicXml = typedXml; + + var typedElement = typedXml.BaseElement.Element("url-picker"); + var dynamicElementByCleanedName = dynamicXml.urlpicker; + + Assert.IsNotNull(typedElement); + Assert.IsNotNull(dynamicElementByCleanedName); + + Assert.AreEqual( + typedElement.Attribute("some-attribute").Value, + dynamicElementByCleanedName.someattribute); + } + + [Test] public void Custom_Extension_Method_Legacy() { var xml = "/media/54/tulips.jpg1024768620888jpg/media/41/hydrangeas.jpg1024768595284jpg"; diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 7f3a4f97c3..551fa86b7d 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -183,6 +183,7 @@ +