diff --git a/.gitignore b/.gitignore index 8fb2789eea..dc6f431550 100644 --- a/.gitignore +++ b/.gitignore @@ -189,3 +189,9 @@ cypress.env.json /src/Umbraco.Web.UI.NetCore/App_Data/TEMP/* /src/Umbraco.Web.UI.NetCore/App_Data/Smidge/Cache/* /src/Umbraco.Web.UI.NetCore/umbraco/logs + +src/Umbraco.Tests.Integration/umbraco/logs/ + +src/Umbraco.Tests.Integration/Views/ + +src/Umbraco.Tests/TEMP/ diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimilarNodeName.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimilarNodeName.cs index 99e824757d..b613ea84aa 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimilarNodeName.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/SimilarNodeName.cs @@ -1,118 +1,225 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; +using System.Text.RegularExpressions; +using static Umbraco.Core.Persistence.Repositories.Implement.SimilarNodeName; namespace Umbraco.Core.Persistence.Repositories.Implement { internal class SimilarNodeName { - private int _numPos = -2; - public int Id { get; set; } public string Name { get; set; } - // cached - reused - public int NumPos - { - get - { - if (_numPos != -2) return _numPos; - - var name = Name; - - // cater nodes with no name. - if (string.IsNullOrWhiteSpace(name)) - return _numPos; - - if (name[name.Length - 1] != ')') - return _numPos = -1; - - var pos = name.LastIndexOf('('); - if (pos < 2 || pos == name.Length - 2) // < 2 and not < 0, because we want at least "x (" - return _numPos = -1; - - return _numPos = pos; - } - } - - // not cached - used only once - public int NumVal - { - get - { - if (NumPos < 0) - throw new InvalidOperationException(); - int num; - if (int.TryParse(Name.Substring(NumPos + 1, Name.Length - 2 - NumPos), out num)) - return num; - return 0; - } - } - - // compare without allocating, nor parsing integers - internal class Comparer : IComparer - { - public int Compare(SimilarNodeName x, SimilarNodeName y) - { - if (x == null) throw new ArgumentNullException("x"); - if (y == null) throw new ArgumentNullException("y"); - - var xpos = x.NumPos; - var ypos = y.NumPos; - - var xname = x.Name; - var yname = y.Name; - - if (xpos < 0 || ypos < 0 || xpos != ypos) - return string.Compare(xname, yname, StringComparison.Ordinal); - - // compare the part before (number) - var n = string.Compare(xname, 0, yname, 0, xpos, StringComparison.Ordinal); - if (n != 0) - return n; - - // compare (number) lengths - var diff = xname.Length - yname.Length; - if (diff != 0) return diff < 0 ? -1 : +1; - - // actually compare (number) - var i = xpos; - while (i < xname.Length - 1) - { - if (xname[i] != yname[i]) - return xname[i] < yname[i] ? -1 : +1; - i++; - } - return 0; - } - } - - // gets a unique name public static string GetUniqueName(IEnumerable names, int nodeId, string nodeName) { - var uniqueNumber = 1; - var uniqueing = false; - foreach (var name in names.OrderBy(x => x, new Comparer())) - { - // ignore self - if (nodeId != 0 && name.Id == nodeId) continue; + var items = names + .Where(x => x.Id != nodeId) // ignore same node + .Select(x => x.Name); - if (uniqueing) + var uniqueName = GetUniqueName(items, nodeName); + + return uniqueName; + } + + public static string GetUniqueName(IEnumerable names, string name) + { + var model = new StructuredName(name); + var items = names + .Where(x => x.InvariantStartsWith(model.Text)) // ignore non-matching names + .Select(x => new StructuredName(x)); + + // name is empty, and there are no other names with suffixes, so just return " (1)" + if (model.IsEmptyName() && !items.Any()) + { + model.Suffix = StructuredName.INITIAL_SUFFIX; + + return model.FullName; + } + + // name is empty, and there are other names with suffixes + if (model.IsEmptyName() && items.SuffixedNameExists()) + { + var emptyNameSuffix = GetSuffixNumber(items); + + if (emptyNameSuffix > 0) { - if (name.NumPos > 0 && name.Name.InvariantStartsWith(nodeName) && name.NumVal == uniqueNumber) - uniqueNumber++; - else - break; - } - else if (name.Name.InvariantEquals(nodeName)) - { - uniqueing = true; + model.Suffix = (uint?)emptyNameSuffix; + + return model.FullName; } } - return uniqueing || string.IsNullOrWhiteSpace(nodeName) - ? string.Concat(nodeName, " (", uniqueNumber.ToString(), ")") - : nodeName; + // no suffix - name without suffix does NOT exist, AND name with suffix does NOT exist + if (!model.Suffix.HasValue && !items.SimpleNameExists(model.Text) && !items.SuffixedNameExists()) + { + model.Suffix = StructuredName.NO_SUFFIX; + + return model.FullName; + } + + // no suffix - name without suffix exists, however name with suffix does NOT exist + if (!model.Suffix.HasValue && items.SimpleNameExists(model.Text) && !items.SuffixedNameExists()) + { + var firstSuffix = GetFirstSuffix(items); + model.Suffix = (uint?)firstSuffix; + + return model.FullName; + } + + // no suffix - name without suffix exists, AND name with suffix does exist + if (!model.Suffix.HasValue && items.SimpleNameExists(model.Text) && items.SuffixedNameExists()) + { + var nextSuffix = GetSuffixNumber(items); + model.Suffix = (uint?)nextSuffix; + + return model.FullName; + } + + // no suffix - name without suffix does NOT exist, however name with suffix exists + if (!model.Suffix.HasValue && !items.SimpleNameExists(model.Text) && items.SuffixedNameExists()) + { + var nextSuffix = GetSuffixNumber(items); + model.Suffix = (uint?)nextSuffix; + + return model.FullName; + } + + // has suffix - name without suffix exists + if (model.Suffix.HasValue && items.SimpleNameExists(model.Text)) + { + var nextSuffix = GetSuffixNumber(items); + model.Suffix = (uint?)nextSuffix; + + return model.FullName; + } + + // has suffix - name without suffix does NOT exist + // a case where the user added the suffix, so add a secondary suffix + if (model.Suffix.HasValue && !items.SimpleNameExists(model.Text)) + { + model.Text = model.FullName; + model.Suffix = StructuredName.NO_SUFFIX; + + // filter items based on full name with suffix + items = items.Where(x => x.Text.InvariantStartsWith(model.FullName)); + var secondarySuffix = GetFirstSuffix(items); + model.Suffix = (uint?)secondarySuffix; + + return model.FullName; + } + + // has suffix - name without suffix also exists, therefore we simply increment + if (model.Suffix.HasValue && items.SimpleNameExists(model.Text)) + { + var nextSuffix = GetSuffixNumber(items); + model.Suffix = (uint?)nextSuffix; + + return model.FullName; + } + + return name; + } + + private static int GetFirstSuffix(IEnumerable items) + { + const int suffixStart = 1; + + if (!items.Any(x => x.Suffix == suffixStart)) + { + // none of the suffixes are the same as suffixStart, so we can use suffixStart! + return suffixStart; + } + + return GetSuffixNumber(items); + } + + private static int GetSuffixNumber(IEnumerable items) + { + int current = 1; + foreach (var item in items.OrderBy(x => x.Suffix)) + { + if (item.Suffix == current) + { + current++; + } + else if (item.Suffix > current) + { + // do nothing - we found our number! + // eg. when suffixes are 1 & 3, then this method is required to generate 2 + break; + } + } + + return current; + } + + internal class StructuredName + { + const string SPACE_CHARACTER = " "; + const string SUFFIXED_PATTERN = @"(.*) \(([1-9]\d*)\)$"; + internal const uint INITIAL_SUFFIX = 1; + internal static readonly uint? NO_SUFFIX = default; + + internal string Text { get; set; } + internal uint ? Suffix { get; set; } + public string FullName + { + get + { + string text = (Text == SPACE_CHARACTER) ? Text.Trim() : Text; + + return Suffix > 0 ? $"{text} ({Suffix})" : text; + } + } + + internal StructuredName(string name) + { + if (string.IsNullOrWhiteSpace(name)) + { + Text = SPACE_CHARACTER; + + return; + } + + var rg = new Regex(SUFFIXED_PATTERN); + var matches = rg.Matches(name); + if (matches.Count > 0) + { + var match = matches[0]; + Text = match.Groups[1].Value; + int number = int.TryParse(match.Groups[2].Value, out number) ? number : 0; + Suffix = (uint?)(number); + + return; + } + else + { + Text = name; + } + } + + internal bool IsEmptyName() + { + return string.IsNullOrWhiteSpace(Text); + } + } + } + + internal static class ListExtensions + { + internal static bool Contains(this IEnumerable items, StructuredName model) + { + return items.Any(x => x.FullName.InvariantEquals(model.FullName)); + } + + internal static bool SimpleNameExists(this IEnumerable items, string name) + { + return items.Any(x => x.FullName.InvariantEquals(name)); + } + + internal static bool SuffixedNameExists(this IEnumerable items) + { + return items.Any(x => x.Suffix.HasValue); } } } diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/SimilarNodeNameTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/SimilarNodeNameTests.cs index 8d8bae2e61..747bb542cd 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/SimilarNodeNameTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/SimilarNodeNameTests.cs @@ -7,72 +7,184 @@ namespace Umbraco.Tests.Integration.Umbraco.Infrastructure.Persistence.Repositor [TestFixture] public class SimilarNodeNameTests { - [TestCase("Alpha", "Alpha", 0)] - [TestCase("Alpha", "ALPHA", +1)] // case is important - [TestCase("Alpha", "Bravo", -1)] - [TestCase("Bravo", "Alpha", +1)] - [TestCase("Alpha (1)", "Alpha (1)", 0)] - [TestCase("Alpha", "Alpha (1)", -1)] - [TestCase("Alpha (1)", "Alpha", +1)] - [TestCase("Alpha (1)", "Alpha (2)", -1)] - [TestCase("Alpha (2)", "Alpha (1)", +1)] - [TestCase("Alpha (2)", "Alpha (10)", -1)] // this is the real stuff - [TestCase("Alpha (10)", "Alpha (2)", +1)] // this is the real stuff - [TestCase("Kilo", "Golf (2)", +1)] - [TestCase("Kilo (1)", "Golf (2)", +1)] - [TestCase("", "", 0)] - [TestCase(null, null, 0)] - public void ComparerTest(string name1, string name2, int expected) - { - var comparer = new SimilarNodeName.Comparer(); - - var result = comparer.Compare(new SimilarNodeName { Name = name1 }, new SimilarNodeName { Name = name2 }); - if (expected == 0) - Assert.AreEqual(0, result); - else if (expected < 0) - Assert.IsTrue(result < 0, "Expected <0 but was " + result); - else if (expected > 0) - Assert.IsTrue(result > 0, "Expected >0 but was " + result); - } - - [Test] - public void OrderByTest() + public void Name_Is_Suffixed() { var names = new[] { - new SimilarNodeName { Id = 1, Name = "Alpha (2)" }, - new SimilarNodeName { Id = 2, Name = "Alpha" }, - new SimilarNodeName { Id = 3, Name = "Golf" }, - new SimilarNodeName { Id = 4, Name = "Zulu" }, - new SimilarNodeName { Id = 5, Name = "Mike" }, - new SimilarNodeName { Id = 6, Name = "Kilo (1)" }, - new SimilarNodeName { Id = 7, Name = "Yankee" }, - new SimilarNodeName { Id = 8, Name = "Kilo" }, - new SimilarNodeName { Id = 9, Name = "Golf (2)" }, - new SimilarNodeName { Id = 10, Name = "Alpha (1)" }, + new SimilarNodeName { Id = 1, Name = "Zulu" } }; - var ordered = names.OrderBy(x => x, new SimilarNodeName.Comparer()).ToArray(); - - var i = 0; - Assert.AreEqual(2, ordered[i++].Id); - Assert.AreEqual(10, ordered[i++].Id); - Assert.AreEqual(1, ordered[i++].Id); - Assert.AreEqual(3, ordered[i++].Id); - Assert.AreEqual(9, ordered[i++].Id); - Assert.AreEqual(8, ordered[i++].Id); - Assert.AreEqual(6, ordered[i++].Id); - Assert.AreEqual(5, ordered[i++].Id); - Assert.AreEqual(7, ordered[i++].Id); - Assert.AreEqual(4, ordered[i++].Id); + var res = SimilarNodeName.GetUniqueName(names, 0, "Zulu"); + Assert.AreEqual("Zulu (1)", res); } + [Test] + public void Suffixed_Name_Is_Incremented() + { + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Zulu" }, + new SimilarNodeName { Id = 2, Name = "Kilo (1)" }, + new SimilarNodeName { Id = 3, Name = "Kilo" }, + }; + + var res = SimilarNodeName.GetUniqueName(names, 0, "Kilo (1)"); + Assert.AreEqual("Kilo (2)", res); + } + + [Test] + public void Lower_Number_Suffix_Is_Inserted() + { + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Golf" }, + new SimilarNodeName { Id = 2, Name = "Golf (2)" }, + }; + + var res = SimilarNodeName.GetUniqueName(names, 0, "Golf"); + Assert.AreEqual("Golf (1)", res); + } + + [Test] + [TestCase(0, "Alpha", "Alpha (3)")] + [TestCase(0, "alpha", "alpha (3)")] + public void Case_Is_Ignored(int nodeId, string nodeName, string expected) + { + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Alpha" }, + new SimilarNodeName { Id = 2, Name = "Alpha (1)" }, + new SimilarNodeName { Id = 3, Name = "Alpha (2)" }, + }; + var res = SimilarNodeName.GetUniqueName(names, nodeId, nodeName); + + Assert.AreEqual(expected, res); + } + + [Test] + public void Empty_List_Causes_Unchanged_Name() + { + var names = new SimilarNodeName[] { }; + + var res = SimilarNodeName.GetUniqueName(names, 0, "Charlie"); + + Assert.AreEqual("Charlie", res); + } + + [Test] + [TestCase(0, "", " (1)")] + [TestCase(0, null, " (1)")] + public void Empty_Name_Is_Suffixed(int nodeId, string nodeName, string expected) + { + var names = new SimilarNodeName[] { }; + + var res = SimilarNodeName.GetUniqueName(names, nodeId, nodeName); + + Assert.AreEqual(expected, res); + } + + [Test] + public void Matching_NoedId_Causes_No_Change() + { + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Kilo (1)" }, + new SimilarNodeName { Id = 2, Name = "Yankee" }, + new SimilarNodeName { Id = 3, Name = "Kilo" }, + }; + + var res = SimilarNodeName.GetUniqueName(names, 1, "Kilo (1)"); + + Assert.AreEqual("Kilo (1)", res); + } + + [Test] + public void Extra_MultiSuffixed_Name_Is_Ignored() + { + // Sequesnce is: Test, Test (1), Test (2) + // Ignore: Test (1) (1) + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Alpha (2)" }, + new SimilarNodeName { Id = 2, Name = "Test" }, + new SimilarNodeName { Id = 3, Name = "Test (1)" }, + new SimilarNodeName { Id = 4, Name = "Test (2)" }, + new SimilarNodeName { Id = 5, Name = "Test (1) (1)" }, + }; + + var res = SimilarNodeName.GetUniqueName(names, 0, "Test"); + + Assert.AreEqual("Test (3)", res); + } + + [Test] + public void Matched_Name_Is_Suffixed() + { + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Test" }, + }; + + var res = SimilarNodeName.GetUniqueName(names, 0, "Test"); + + Assert.AreEqual("Test (1)", res); + } + + [Test] + public void MultiSuffixed_Name_Is_Icremented() + { + // "Test (1)" is treated as the "original" version of the name. + // "Test (1) (1)" is the suffixed result of a copy, and therefore is incremented + // Hence this test result should be the same as Suffixed_Name_Is_Incremented + var names = new[] + { + new SimilarNodeName { Id = 1, Name = "Test (1)" }, + new SimilarNodeName { Id = 2, Name = "Test (1) (1)" }, + }; + + var res = SimilarNodeName.GetUniqueName(names, 0, "Test (1) (1)"); + + Assert.AreEqual("Test (1) (2)", res); + } + + [Test] + public void Suffixed_Name_Causes_Secondary_Suffix() + { + var names = new[] + { + new SimilarNodeName { Id = 6, Name = "Alpha (1)" } + }; + var res = SimilarNodeName.GetUniqueName(names, 0, "Alpha (1)"); + + Assert.AreEqual("Alpha (1) (1)", res); + } + + + [TestCase("Test (0)", "Test (0) (1)")] + [TestCase("Test (-1)", "Test (-1) (1)")] + [TestCase("Test (1) (-1)", "Test (1) (-1) (1)")] + public void NonPositive_Suffix_Is_Ignored(string suffix, string expected) + { + var names = new[] + { + new SimilarNodeName { Id = 6, Name = suffix } + }; + var res = SimilarNodeName.GetUniqueName(names, 0, suffix); + + Assert.AreEqual(expected, res); + } + + + + /* Original Tests - Can be deleted, as new tests cover all cases */ + [TestCase(0, "Charlie", "Charlie")] [TestCase(0, "Zulu", "Zulu (1)")] [TestCase(0, "Golf", "Golf (1)")] [TestCase(0, "Kilo", "Kilo (2)")] [TestCase(0, "Alpha", "Alpha (3)")] - [TestCase(0, "Kilo (1)", "Kilo (1) (1)")] // though... we might consider "Kilo (2)" + //[TestCase(0, "Kilo (1)", "Kilo (1) (1)")] // though... we might consider "Kilo (2)" + [TestCase(0, "Kilo (1)", "Kilo (2)")] // names[] contains "Kilo" AND "Kilo (1)", which implies that result should be "Kilo (2)" [TestCase(6, "Kilo (1)", "Kilo (1)")] // because of the id [TestCase(0, "alpha", "alpha (3)")] [TestCase(0, "", " (1)")]