From 9f04a9de9c58fc9c834a7b85bd3ce65de469bebd Mon Sep 17 00:00:00 2001 From: Mundairson Date: Sun, 1 Jul 2018 11:23:38 +0100 Subject: [PATCH 1/6] Refactored simple conversion methods. Added unit tests to cover the methods. --- .../FrontEnd/UmbracoHelperTests.cs | 269 +++++++++++++++++- src/Umbraco.Web/UmbracoHelper.cs | 68 +++-- 2 files changed, 304 insertions(+), 33 deletions(-) diff --git a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs index bc5971a377..271aaae337 100644 --- a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs +++ b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs @@ -1,6 +1,14 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Text; +using LightInject; +using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Cache; +using Umbraco.Core.Composing; +using Umbraco.Core.Logging; +using Umbraco.Tests.TestHelpers; using Umbraco.Web; namespace Umbraco.Tests.FrontEnd @@ -8,6 +16,33 @@ namespace Umbraco.Tests.FrontEnd [TestFixture] public class UmbracoHelperTests { + [SetUp] + public void SetUp() + { + // fixme - bad in a unit test - but Udi has a static ctor that wants it?! + var container = new Mock(); + var globalSettings = SettingsForTests.GenerateMockGlobalSettings(); + + container + .Setup(x => x.GetInstance(typeof(TypeLoader))) + .Returns(new TypeLoader( + NullCacheProvider.Instance, + globalSettings, + new ProfilingLogger(Mock.Of(), Mock.Of()) + ) + ); + + Current.Container = container.Object; + + Udi.ResetUdiTypes(); + } + + [TearDown] + public void TearDown() + { + Current.Reset(); + } + [Test] public void Truncate_Simple() { @@ -68,7 +103,7 @@ namespace Umbraco.Tests.FrontEnd key2 = "value2", Key3 = "Value3", keY4 = "valuE4" - }; + }; var encryptedRouteString = UmbracoHelper.CreateEncryptedRouteString("FormController", "FormAction", "", additionalRouteValues); var result = encryptedRouteString.DecryptWithMachineKey(); var expectedResult = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; @@ -146,7 +181,7 @@ namespace Umbraco.Tests.FrontEnd { var text = "Hello world, this is some text with a link"; - string [] tags = {"b"}; + string[] tags = { "b" }; var helper = new UmbracoHelper(); @@ -166,5 +201,233 @@ namespace Umbraco.Tests.FrontEnd Assert.AreEqual("Hello world, is some text with a link", result); } + + // ------- Int32 conversion tests + [Test] + public static void Converting_boxed_34_to_an_int_returns_34() + { + // Arrange + const int sample = 34; + + // Act + bool success = UmbracoHelper.ConvertIdObjectToInt( + sample, + out int result + ); + + // Assert + Assert.IsTrue(success); + Assert.That(result, Is.EqualTo(34)); + } + + [Test] + public static void Converting_string_54_to_an_int_returns_54() + { + // Arrange + const string sample = "54"; + + // Act + bool success = UmbracoHelper.ConvertIdObjectToInt( + sample, + out int result + ); + + // Assert + Assert.IsTrue(success); + Assert.That(result, Is.EqualTo(54)); + } + + [Test] + public static void Converting_hello_to_an_int_returns_false() + { + // Arrange + const string sample = "Hello"; + + // Act + bool success = UmbracoHelper.ConvertIdObjectToInt( + sample, + out int result + ); + + // Assert + Assert.IsFalse(success); + Assert.That(result, Is.EqualTo(0)); + } + + [Test] + public static void Converting_unsupported_object_to_an_int_returns_false() + { + // Arrange + var clearlyWillNotConvertToInt = new StringBuilder(0); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToInt( + clearlyWillNotConvertToInt, + out int result + ); + + // Assert + Assert.IsFalse(success); + Assert.That(result, Is.EqualTo(0)); + } + + // ------- GUID conversion tests + [Test] + public static void Converting_boxed_guid_to_a_guid_returns_original_guid_value() + { + // Arrange + Guid sample = Guid.NewGuid(); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToGuid( + sample, + out Guid result + ); + + // Assert + Assert.IsTrue(success); + Assert.That(result, Is.EqualTo(sample)); + } + + [Test] + public static void Converting_string_guid_to_a_guid_returns_original_guid_value() + { + // Arrange + Guid sample = Guid.NewGuid(); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToGuid( + sample.ToString(), + out Guid result + ); + + // Assert + Assert.IsTrue(success); + Assert.That(result, Is.EqualTo(sample)); + } + + [Test] + public static void Converting_hello_to_a_guid_returns_false() + { + // Arrange + const string sample = "Hello"; + + // Act + bool success = UmbracoHelper.ConvertIdObjectToGuid( + sample, + out Guid result + ); + + // Assert + Assert.IsFalse(success); + Assert.That(result, Is.EqualTo(new Guid("00000000-0000-0000-0000-000000000000"))); + } + + [Test] + public static void Converting_unsupported_object_to_a_guid_returns_false() + { + // Arrange + var clearlyWillNotConvertToGuid = new StringBuilder(0); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToGuid( + clearlyWillNotConvertToGuid, + out Guid result + ); + + // Assert + Assert.IsFalse(success); + Assert.That(result, Is.EqualTo(new Guid("00000000-0000-0000-0000-000000000000"))); + } + + // ------- UDI Conversion Tests + /// + /// This requires PluginManager.Current to be initialised before + /// running. + /// + [Test] + public static void Converting_boxed_udi_to_a_udi_returns_original_udi_value() + { + // Arrange + Udi.ResetUdiTypes(); + Udi sample = new GuidUdi(Constants.UdiEntityType.AnyGuid, Guid.NewGuid()); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToUdi( + sample, + out Udi result + ); + + // Assert + Assert.IsTrue(success); + Assert.That(result, Is.EqualTo(sample)); + } + + /// + /// This requires PluginManager.Current to be initialised before + /// running. + /// + [Test] + public static void Converting_string_udi_to_a_udi_returns_original_udi_value() + { + // Arrange + Udi.ResetUdiTypes(); + Udi sample = new GuidUdi(Constants.UdiEntityType.AnyGuid, Guid.NewGuid()); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToUdi( + sample.ToString(), + out Udi result + ); + + // Assert + Assert.IsTrue(success, "Conversion of UDI failed."); + Assert.That(result, Is.EqualTo(sample)); + } + + /// + /// This requires PluginManager.Current to be initialised before + /// running. + /// + [Test] + public static void Converting_hello_to_a_udi_returns_false() + { + // Arrange + Udi.ResetUdiTypes(); + const string sample = "Hello"; + + // Act + bool success = UmbracoHelper.ConvertIdObjectToUdi( + sample, + out Udi result + ); + + // Assert + Assert.IsFalse(success); + Assert.That(result, Is.Null); + } + + /// + /// This requires PluginManager.Current to be initialised before + /// running. + /// + [Test] + public static void Converting_unsupported_object_to_a_udi_returns_false() + { + // Arrange + Udi.ResetUdiTypes(); + + var clearlyWillNotConvertToGuid = new StringBuilder(0); + + // Act + bool success = UmbracoHelper.ConvertIdObjectToUdi( + clearlyWillNotConvertToGuid, + out Udi result + ); + + // Assert + Assert.IsFalse(success); + Assert.That(result, Is.Null); + } } } diff --git a/src/Umbraco.Web/UmbracoHelper.cs b/src/Umbraco.Web/UmbracoHelper.cs index ca76924b23..37847c6e6f 100644 --- a/src/Umbraco.Web/UmbracoHelper.cs +++ b/src/Umbraco.Web/UmbracoHelper.cs @@ -601,37 +601,40 @@ namespace Umbraco.Web return ContentQuery.ContentAtRoot(); } - private static bool ConvertIdObjectToInt(object id, out int intId) + /// Had to change to internal for testing. + internal static bool ConvertIdObjectToInt(object id, out int intId) { - var s = id as string; - if (s != null) + switch (id) { - return int.TryParse(s, out intId); - } + case string s: + return int.TryParse(s, out intId); - if (id is int) - { - intId = (int) id; - return true; + case int i: + intId = i; + return true; + + default: + intId = default; + return false; } - intId = default(int); - return false; } - private static bool ConvertIdObjectToGuid(object id, out Guid guidId) + /// Had to change to internal for testing. + internal static bool ConvertIdObjectToGuid(object id, out Guid guidId) { - var s = id as string; - if (s != null) + switch (id) { - return Guid.TryParse(s, out guidId); + case string s: + return Guid.TryParse(s, out guidId); + + case Guid g: + guidId = g; + return true; + + default: + guidId = default; + return false; } - if (id is Guid) - { - guidId = (Guid) id; - return true; - } - guidId = default(Guid); - return false; } private static bool ConvertIdsObjectToInts(IEnumerable ids, out IEnumerable intIds) @@ -665,17 +668,22 @@ namespace Umbraco.Web return true; } - private static bool ConvertIdObjectToUdi(object id, out Udi guidId) + /// Had to change to internal for testing. + internal static bool ConvertIdObjectToUdi(object id, out Udi guidId) { - if (id is string s) - return Udi.TryParse(s, out guidId); - if (id is Udi) + switch (id) { - guidId = (Udi) id; - return true; + case string s: + return Udi.TryParse(s, out guidId); + + case Udi u: + guidId = u; + return true; + + default: + guidId = default; + return false; } - guidId = null; - return false; } #endregion From 7caf7e6a787475d8c37822661f1f8b36be4a1a48 Mon Sep 17 00:00:00 2001 From: Mundairson Date: Sun, 1 Jul 2018 11:24:09 +0100 Subject: [PATCH 2/6] Removed unneeded usings. --- src/Umbraco.Web/UmbracoHelper.cs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Web/UmbracoHelper.cs b/src/Umbraco.Web/UmbracoHelper.cs index 37847c6e6f..45e8fdf050 100644 --- a/src/Umbraco.Web/UmbracoHelper.cs +++ b/src/Umbraco.Web/UmbracoHelper.cs @@ -1,24 +1,20 @@ using System; +using System.Collections.Generic; using System.ComponentModel; +using System.Linq; using System.Web; -using System.Web.Security; using System.Xml.XPath; using Umbraco.Core; -using Umbraco.Core.Dictionary; -using Umbraco.Core.Security; -using Umbraco.Core.Services; -using Umbraco.Core.Xml; -using Umbraco.Web.Routing; -using Umbraco.Web.Security; -using System.Collections.Generic; -using System.IO; -using System.Linq; -using System.Web.Mvc; using Umbraco.Core.Cache; +using Umbraco.Core.Dictionary; using Umbraco.Core.Exceptions; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; +using Umbraco.Core.Services; +using Umbraco.Core.Xml; using Umbraco.Web.Composing; +using Umbraco.Web.Routing; +using Umbraco.Web.Security; namespace Umbraco.Web { @@ -894,7 +890,7 @@ namespace Umbraco.Web { return StringUtilities.ReplaceLineBreaksForHtml(text); } - + /// /// Generates a hash based on the text string passed in. This method will detect the /// security requirements (is FIPS enabled) and return an appropriate hash. @@ -1059,7 +1055,7 @@ namespace Umbraco.Web } #endregion - + /// /// This is used in methods like BeginUmbracoForm and SurfaceAction to generate an encrypted string which gets submitted in a request for which /// Umbraco can decrypt during the routing process in order to delegate the request to a specific MVC Controller. From 1879cfe029b4c305df0ca27c269e513a68b29227 Mon Sep 17 00:00:00 2001 From: Mundairson Date: Sun, 1 Jul 2018 15:36:55 +0100 Subject: [PATCH 3/6] Restricted the UDI set up and tear down to only the UDI tests. --- .../FrontEnd/UmbracoHelperTests.cs | 94 ++++++++----------- 1 file changed, 40 insertions(+), 54 deletions(-) diff --git a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs index 271aaae337..aab30df493 100644 --- a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs +++ b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs @@ -16,33 +16,6 @@ namespace Umbraco.Tests.FrontEnd [TestFixture] public class UmbracoHelperTests { - [SetUp] - public void SetUp() - { - // fixme - bad in a unit test - but Udi has a static ctor that wants it?! - var container = new Mock(); - var globalSettings = SettingsForTests.GenerateMockGlobalSettings(); - - container - .Setup(x => x.GetInstance(typeof(TypeLoader))) - .Returns(new TypeLoader( - NullCacheProvider.Instance, - globalSettings, - new ProfilingLogger(Mock.Of(), Mock.Of()) - ) - ); - - Current.Container = container.Object; - - Udi.ResetUdiTypes(); - } - - [TearDown] - public void TearDown() - { - Current.Reset(); - } - [Test] public void Truncate_Simple() { @@ -341,15 +314,11 @@ namespace Umbraco.Tests.FrontEnd } // ------- UDI Conversion Tests - /// - /// This requires PluginManager.Current to be initialised before - /// running. - /// [Test] - public static void Converting_boxed_udi_to_a_udi_returns_original_udi_value() + public void Converting_boxed_udi_to_a_udi_returns_original_udi_value() { // Arrange - Udi.ResetUdiTypes(); + SetUpUdiTests(); Udi sample = new GuidUdi(Constants.UdiEntityType.AnyGuid, Guid.NewGuid()); // Act @@ -359,19 +328,17 @@ namespace Umbraco.Tests.FrontEnd ); // Assert + TearDownUdiTests(); + Assert.IsTrue(success); Assert.That(result, Is.EqualTo(sample)); } - /// - /// This requires PluginManager.Current to be initialised before - /// running. - /// [Test] - public static void Converting_string_udi_to_a_udi_returns_original_udi_value() + public void Converting_string_udi_to_a_udi_returns_original_udi_value() { // Arrange - Udi.ResetUdiTypes(); + SetUpUdiTests(); Udi sample = new GuidUdi(Constants.UdiEntityType.AnyGuid, Guid.NewGuid()); // Act @@ -381,42 +348,37 @@ namespace Umbraco.Tests.FrontEnd ); // Assert + TearDownUdiTests(); + Assert.IsTrue(success, "Conversion of UDI failed."); Assert.That(result, Is.EqualTo(sample)); } - /// - /// This requires PluginManager.Current to be initialised before - /// running. - /// [Test] - public static void Converting_hello_to_a_udi_returns_false() + public void Converting_hello_to_a_udi_returns_false() { // Arrange - Udi.ResetUdiTypes(); - const string sample = "Hello"; + SetUpUdiTests(); + const string SAMPLE = "Hello"; // Act bool success = UmbracoHelper.ConvertIdObjectToUdi( - sample, + SAMPLE, out Udi result ); // Assert + TearDownUdiTests(); + Assert.IsFalse(success); Assert.That(result, Is.Null); } - /// - /// This requires PluginManager.Current to be initialised before - /// running. - /// [Test] - public static void Converting_unsupported_object_to_a_udi_returns_false() + public void Converting_unsupported_object_to_a_udi_returns_false() { // Arrange - Udi.ResetUdiTypes(); - + SetUpUdiTests(); var clearlyWillNotConvertToGuid = new StringBuilder(0); // Act @@ -426,8 +388,32 @@ namespace Umbraco.Tests.FrontEnd ); // Assert + TearDownUdiTests(); + Assert.IsFalse(success); Assert.That(result, Is.Null); } + + public void SetUpUdiTests() + { + // fixme - bad in a unit test - but Udi has a static ctor that wants it?! + var container = new Mock(); + var globalSettings = SettingsForTests.GenerateMockGlobalSettings(); + + container + .Setup(x => x.GetInstance(typeof(TypeLoader))) + .Returns(new TypeLoader( + NullCacheProvider.Instance, + globalSettings, + new ProfilingLogger(Mock.Of(), Mock.Of()) + ) + ); + + Current.Container = container.Object; + + Udi.ResetUdiTypes(); + } + + public void TearDownUdiTests() => Current.Reset(); } } From 082bf484ed9e013eb2d715a1e96210f1548298ce Mon Sep 17 00:00:00 2001 From: Mundairson Date: Sun, 1 Jul 2018 16:20:02 +0100 Subject: [PATCH 4/6] Clean tests. --- .../FrontEnd/UmbracoHelperTests.cs | 111 +++++++++--------- 1 file changed, 57 insertions(+), 54 deletions(-) diff --git a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs index aab30df493..92fa791cfa 100644 --- a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs +++ b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs @@ -16,59 +16,50 @@ namespace Umbraco.Tests.FrontEnd [TestFixture] public class UmbracoHelperTests { - [Test] - public void Truncate_Simple() - { - var text = "Hello world, this is some text with a link"; + private const string SAMPLE = "Hello world, this is some text with a link"; + [Test] + public static void Truncate_Simple() + { var helper = new UmbracoHelper(); - var result = helper.Truncate(text, 25).ToString(); - - Assert.AreEqual("Hello world, this is some…", result); - } - - /// - /// If a truncated string ends with a space, we should trim the space before appending the ellipsis. - /// - [Test] - public void Truncate_Simple_With_Trimming() - { - var text = "Hello world, this is some text with a link"; - - var helper = new UmbracoHelper(); - - var result = helper.Truncate(text, 26).ToString(); + var result = helper.Truncate(SAMPLE, 25).ToString(); Assert.AreEqual("Hello world, this is some…", result); } [Test] - public void Truncate_Inside_Word() + public static void When_truncating_a_string_ending_with_a_space_we_should_trim_the_space_before_appending_the_ellipsis() { - var text = "Hello world, this is some text with a link"; - var helper = new UmbracoHelper(); - var result = helper.Truncate(text, 24).ToString(); + var result = helper.Truncate(SAMPLE, 26).ToString(); + + Assert.AreEqual("Hello world, this is some…", result); + } + + [Test] + public static void Truncate_Inside_Word() + { + var helper = new UmbracoHelper(); + + var result = helper.Truncate(SAMPLE, 24).ToString(); Assert.AreEqual("Hello world, this is som…", result); } [Test] - public void Truncate_With_Tag() + public static void Truncate_With_Tag() { - var text = "Hello world, this is some text with a link"; - var helper = new UmbracoHelper(); - var result = helper.Truncate(text, 35).ToString(); + var result = helper.Truncate(SAMPLE, 35).ToString(); Assert.AreEqual("Hello world, this is some text with…", result); } [Test] - public void Create_Encrypted_RouteString_From_Anonymous_Object() + public static void Create_Encrypted_RouteString_From_Anonymous_Object() { var additionalRouteValues = new { @@ -77,15 +68,23 @@ namespace Umbraco.Tests.FrontEnd Key3 = "Value3", keY4 = "valuE4" }; - var encryptedRouteString = UmbracoHelper.CreateEncryptedRouteString("FormController", "FormAction", "", additionalRouteValues); - var result = encryptedRouteString.DecryptWithMachineKey(); - var expectedResult = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; - Assert.AreEqual(expectedResult, result); + var encryptedRouteString = UmbracoHelper.CreateEncryptedRouteString( + "FormController", + "FormAction", + "", + additionalRouteValues + ); + + var result = encryptedRouteString.DecryptWithMachineKey(); + + const string EXPECTED_RESULT = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; + + Assert.AreEqual(EXPECTED_RESULT, result); } [Test] - public void Create_Encrypted_RouteString_From_Dictionary() + public static void Create_Encrypted_RouteString_From_Dictionary() { var additionalRouteValues = new Dictionary() { @@ -94,29 +93,35 @@ namespace Umbraco.Tests.FrontEnd {"Key3", "Value3"}, {"keY4", "valuE4"} }; - var encryptedRouteString = UmbracoHelper.CreateEncryptedRouteString("FormController", "FormAction", "", additionalRouteValues); - var result = encryptedRouteString.DecryptWithMachineKey(); - var expectedResult = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; - Assert.AreEqual(expectedResult, result); + var encryptedRouteString = UmbracoHelper.CreateEncryptedRouteString( + "FormController", + "FormAction", + "", + additionalRouteValues + ); + + var result = encryptedRouteString.DecryptWithMachineKey(); + + const string EXPECTED_RESULT = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; + + Assert.AreEqual(EXPECTED_RESULT, result); } [Test] - public void Truncate_By_Words() + public static void Truncate_By_Words() { - var text = "Hello world, this is some text with a link"; - var helper = new UmbracoHelper(); - var result = helper.TruncateByWords(text, 4).ToString(); + var result = helper.TruncateByWords(SAMPLE, 4).ToString(); Assert.AreEqual("Hello world, this is…", result); } [Test] - public void Truncate_By_Words_With_Tag() + public static void Truncate_By_Words_With_Tag() { - var text = "Hello world, this is some text with a link"; + const string text = "Hello world, this is some text with a link"; var helper = new UmbracoHelper(); @@ -126,21 +131,19 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public void Truncate_By_Words_Mid_Tag() + public static void Truncate_By_Words_Mid_Tag() { - var text = "Hello world, this is some text with a link"; - var helper = new UmbracoHelper(); - var result = helper.TruncateByWords(text, 7).ToString(); + var result = helper.TruncateByWords(SAMPLE, 7).ToString(); Assert.AreEqual("Hello world, this is some text with…", result); } [Test] - public void Strip_All_Html() + public static void Strip_All_Html() { - var text = "Hello world, this is some text with a link"; + const string text = "Hello world, this is some text with a link"; var helper = new UmbracoHelper(); @@ -150,9 +153,9 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public void Strip_Specific_Html() + public static void Strip_Specific_Html() { - var text = "Hello world, this is some text with a link"; + const string text = "Hello world, this is some text with a link"; string[] tags = { "b" }; @@ -164,9 +167,9 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public void Strip_Invalid_Html() + public static void Strip_Invalid_Html() { - var text = "Hello world, is some text with a link"; + const string text = "Hello world, is some text with a link"; var helper = new UmbracoHelper(); From d4d90e1d4d6f4335d3408a2aeafdac09a3884d21 Mon Sep 17 00:00:00 2001 From: Mundairson Date: Sun, 1 Jul 2018 16:33:48 +0100 Subject: [PATCH 5/6] Corrected test name to match semantics. --- src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs index 92fa791cfa..009274e8c7 100644 --- a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs +++ b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs @@ -29,7 +29,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void When_truncating_a_string_ending_with_a_space_we_should_trim_the_space_before_appending_the_ellipsis() + public static void When_truncating_a_string_ends_with_a_space_we_should_trim_the_space_before_appending_the_ellipsis() { var helper = new UmbracoHelper(); From 85c9e66cee7bad05dd9f61950bdacbc3ccfff7bc Mon Sep 17 00:00:00 2001 From: Mundairson Date: Wed, 11 Jul 2018 00:12:18 +0100 Subject: [PATCH 6/6] Implemented changes from first PR code review. --- .../FrontEnd/UmbracoHelperTests.cs | 93 ++++++++++--------- src/Umbraco.Web/UmbracoHelper.cs | 3 - 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs index 009274e8c7..0c05df2342 100644 --- a/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs +++ b/src/Umbraco.Tests/FrontEnd/UmbracoHelperTests.cs @@ -16,24 +16,25 @@ namespace Umbraco.Tests.FrontEnd [TestFixture] public class UmbracoHelperTests { - private const string SAMPLE = "Hello world, this is some text with a link"; + private const string SampleWithAnchorElement = "Hello world, this is some text with a link"; + private const string SampleWithBoldAndAnchorElements = "Hello world, this is some text with a link"; [Test] public static void Truncate_Simple() { var helper = new UmbracoHelper(); - var result = helper.Truncate(SAMPLE, 25).ToString(); + var result = helper.Truncate(SampleWithAnchorElement, 25).ToString(); Assert.AreEqual("Hello world, this is some…", result); } [Test] - public static void When_truncating_a_string_ends_with_a_space_we_should_trim_the_space_before_appending_the_ellipsis() + public static void When_Truncating_A_String_Ends_With_A_Space_We_Should_Trim_The_Space_Before_Appending_The_Ellipsis() { var helper = new UmbracoHelper(); - var result = helper.Truncate(SAMPLE, 26).ToString(); + var result = helper.Truncate(SampleWithAnchorElement, 26).ToString(); Assert.AreEqual("Hello world, this is some…", result); } @@ -43,7 +44,7 @@ namespace Umbraco.Tests.FrontEnd { var helper = new UmbracoHelper(); - var result = helper.Truncate(SAMPLE, 24).ToString(); + var result = helper.Truncate(SampleWithAnchorElement, 24).ToString(); Assert.AreEqual("Hello world, this is som…", result); } @@ -53,7 +54,7 @@ namespace Umbraco.Tests.FrontEnd { var helper = new UmbracoHelper(); - var result = helper.Truncate(SAMPLE, 35).ToString(); + var result = helper.Truncate(SampleWithAnchorElement, 35).ToString(); Assert.AreEqual("Hello world, this is some text with…", result); } @@ -78,9 +79,9 @@ namespace Umbraco.Tests.FrontEnd var result = encryptedRouteString.DecryptWithMachineKey(); - const string EXPECTED_RESULT = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; + const string expectedResult = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; - Assert.AreEqual(EXPECTED_RESULT, result); + Assert.AreEqual(expectedResult, result); } [Test] @@ -103,9 +104,9 @@ namespace Umbraco.Tests.FrontEnd var result = encryptedRouteString.DecryptWithMachineKey(); - const string EXPECTED_RESULT = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; + const string expectedResult = "c=FormController&a=FormAction&ar=&key1=value1&key2=value2&Key3=Value3&keY4=valuE4"; - Assert.AreEqual(EXPECTED_RESULT, result); + Assert.AreEqual(expectedResult, result); } [Test] @@ -113,7 +114,7 @@ namespace Umbraco.Tests.FrontEnd { var helper = new UmbracoHelper(); - var result = helper.TruncateByWords(SAMPLE, 4).ToString(); + var result = helper.TruncateByWords(SampleWithAnchorElement, 4).ToString(); Assert.AreEqual("Hello world, this is…", result); } @@ -121,11 +122,9 @@ namespace Umbraco.Tests.FrontEnd [Test] public static void Truncate_By_Words_With_Tag() { - const string text = "Hello world, this is some text with a link"; - var helper = new UmbracoHelper(); - var result = helper.TruncateByWords(text, 4).ToString(); + var result = helper.TruncateByWords(SampleWithBoldAndAnchorElements, 4).ToString(); Assert.AreEqual("Hello world, this is…", result); } @@ -135,7 +134,7 @@ namespace Umbraco.Tests.FrontEnd { var helper = new UmbracoHelper(); - var result = helper.TruncateByWords(SAMPLE, 7).ToString(); + var result = helper.TruncateByWords(SampleWithAnchorElement, 7).ToString(); Assert.AreEqual("Hello world, this is some text with…", result); } @@ -143,11 +142,9 @@ namespace Umbraco.Tests.FrontEnd [Test] public static void Strip_All_Html() { - const string text = "Hello world, this is some text with a link"; - var helper = new UmbracoHelper(); - var result = helper.StripHtml(text, null).ToString(); + var result = helper.StripHtml(SampleWithBoldAndAnchorElements, null).ToString(); Assert.AreEqual("Hello world, this is some text with a link", result); } @@ -155,13 +152,11 @@ namespace Umbraco.Tests.FrontEnd [Test] public static void Strip_Specific_Html() { - const string text = "Hello world, this is some text with a link"; - string[] tags = { "b" }; var helper = new UmbracoHelper(); - var result = helper.StripHtml(text, tags).ToString(); + var result = helper.StripHtml(SampleWithBoldAndAnchorElements, tags).ToString(); Assert.AreEqual("Hello world, this is some text with a link", result); } @@ -180,7 +175,7 @@ namespace Umbraco.Tests.FrontEnd // ------- Int32 conversion tests [Test] - public static void Converting_boxed_34_to_an_int_returns_34() + public static void Converting_Boxed_34_To_An_Int_Returns_34() { // Arrange const int sample = 34; @@ -197,7 +192,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void Converting_string_54_to_an_int_returns_54() + public static void Converting_String_54_To_An_Int_Returns_54() { // Arrange const string sample = "54"; @@ -214,7 +209,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void Converting_hello_to_an_int_returns_false() + public static void Converting_Hello_To_An_Int_Returns_False() { // Arrange const string sample = "Hello"; @@ -231,7 +226,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void Converting_unsupported_object_to_an_int_returns_false() + public static void Converting_Unsupported_Object_To_An_Int_Returns_False() { // Arrange var clearlyWillNotConvertToInt = new StringBuilder(0); @@ -249,7 +244,7 @@ namespace Umbraco.Tests.FrontEnd // ------- GUID conversion tests [Test] - public static void Converting_boxed_guid_to_a_guid_returns_original_guid_value() + public static void Converting_Boxed_Guid_To_A_Guid_Returns_Original_Guid_Value() { // Arrange Guid sample = Guid.NewGuid(); @@ -266,7 +261,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void Converting_string_guid_to_a_guid_returns_original_guid_value() + public static void Converting_String_Guid_To_A_Guid_Returns_Original_Guid_Value() { // Arrange Guid sample = Guid.NewGuid(); @@ -283,7 +278,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void Converting_hello_to_a_guid_returns_false() + public static void Converting_Hello_To_A_Guid_Returns_False() { // Arrange const string sample = "Hello"; @@ -300,7 +295,7 @@ namespace Umbraco.Tests.FrontEnd } [Test] - public static void Converting_unsupported_object_to_a_guid_returns_false() + public static void Converting_Unsupported_Object_To_A_Guid_Returns_False() { // Arrange var clearlyWillNotConvertToGuid = new StringBuilder(0); @@ -318,10 +313,12 @@ namespace Umbraco.Tests.FrontEnd // ------- UDI Conversion Tests [Test] - public void Converting_boxed_udi_to_a_udi_returns_original_udi_value() + public void Converting_Boxed_Udi_To_A_Udi_Returns_Original_Udi_Value() { // Arrange - SetUpUdiTests(); + SetUpDependencyContainer(); + Udi.ResetUdiTypes(); + Udi sample = new GuidUdi(Constants.UdiEntityType.AnyGuid, Guid.NewGuid()); // Act @@ -331,17 +328,19 @@ namespace Umbraco.Tests.FrontEnd ); // Assert - TearDownUdiTests(); + ResetDependencyContainer(); Assert.IsTrue(success); Assert.That(result, Is.EqualTo(sample)); } [Test] - public void Converting_string_udi_to_a_udi_returns_original_udi_value() + public void Converting_String_Udi_To_A_Udi_Returns_Original_Udi_Value() { // Arrange - SetUpUdiTests(); + SetUpDependencyContainer(); + Udi.ResetUdiTypes(); + Udi sample = new GuidUdi(Constants.UdiEntityType.AnyGuid, Guid.NewGuid()); // Act @@ -351,17 +350,19 @@ namespace Umbraco.Tests.FrontEnd ); // Assert - TearDownUdiTests(); + ResetDependencyContainer(); - Assert.IsTrue(success, "Conversion of UDI failed."); + Assert.IsTrue(success); Assert.That(result, Is.EqualTo(sample)); } [Test] - public void Converting_hello_to_a_udi_returns_false() + public void Converting_Hello_To_A_Udi_Returns_False() { // Arrange - SetUpUdiTests(); + SetUpDependencyContainer(); + Udi.ResetUdiTypes(); + const string SAMPLE = "Hello"; // Act @@ -371,17 +372,19 @@ namespace Umbraco.Tests.FrontEnd ); // Assert - TearDownUdiTests(); + ResetDependencyContainer(); Assert.IsFalse(success); Assert.That(result, Is.Null); } [Test] - public void Converting_unsupported_object_to_a_udi_returns_false() + public void Converting_Unsupported_Object_To_A_Udi_Returns_False() { // Arrange - SetUpUdiTests(); + SetUpDependencyContainer(); + Udi.ResetUdiTypes(); + var clearlyWillNotConvertToGuid = new StringBuilder(0); // Act @@ -391,13 +394,13 @@ namespace Umbraco.Tests.FrontEnd ); // Assert - TearDownUdiTests(); + ResetDependencyContainer(); Assert.IsFalse(success); Assert.That(result, Is.Null); } - public void SetUpUdiTests() + private void SetUpDependencyContainer() { // fixme - bad in a unit test - but Udi has a static ctor that wants it?! var container = new Mock(); @@ -413,10 +416,8 @@ namespace Umbraco.Tests.FrontEnd ); Current.Container = container.Object; - - Udi.ResetUdiTypes(); } - public void TearDownUdiTests() => Current.Reset(); + private void ResetDependencyContainer() => Current.Reset(); } } diff --git a/src/Umbraco.Web/UmbracoHelper.cs b/src/Umbraco.Web/UmbracoHelper.cs index 45e8fdf050..0ccf18157a 100644 --- a/src/Umbraco.Web/UmbracoHelper.cs +++ b/src/Umbraco.Web/UmbracoHelper.cs @@ -597,7 +597,6 @@ namespace Umbraco.Web return ContentQuery.ContentAtRoot(); } - /// Had to change to internal for testing. internal static bool ConvertIdObjectToInt(object id, out int intId) { switch (id) @@ -615,7 +614,6 @@ namespace Umbraco.Web } } - /// Had to change to internal for testing. internal static bool ConvertIdObjectToGuid(object id, out Guid guidId) { switch (id) @@ -664,7 +662,6 @@ namespace Umbraco.Web return true; } - /// Had to change to internal for testing. internal static bool ConvertIdObjectToUdi(object id, out Udi guidId) { switch (id)