Merge pull request #9221 from anthonydotnet/unique-nodename-refactor

Unique node name unit test fix & refactor
This commit is contained in:
Bjarke Berg
2020-10-23 07:16:25 +02:00
committed by GitHub
3 changed files with 378 additions and 153 deletions

6
.gitignore vendored
View File

@@ -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/

View File

@@ -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<SimilarNodeName>
{
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<SimilarNodeName> 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<string> 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<StructuredName> 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<StructuredName> 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<StructuredName> items, StructuredName model)
{
return items.Any(x => x.FullName.InvariantEquals(model.FullName));
}
internal static bool SimpleNameExists(this IEnumerable<StructuredName> items, string name)
{
return items.Any(x => x.FullName.InvariantEquals(name));
}
internal static bool SuffixedNameExists(this IEnumerable<StructuredName> items)
{
return items.Any(x => x.Suffix.HasValue);
}
}
}

View File

@@ -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)")]