Less parsing, allocation & LINQ when splitting strings (#18048)

* Less parsing, allocation & LINQ when splitting strings

* Added unit test verifying refactoring of GetIdsFromPathReversed.

---------

Co-authored-by: Andy Butland <[email protected]>

Author: Henr1k80

src/Umbraco.Core/DynamicRoot/Origin/SiteDynamicRootOriginFinder.cs

@@ -33,11 +33,10 @@ public SiteDynamicRootOriginFinder(IEntityService entityService, IDomainService
             return null;
         }
 
-
-        IEnumerable<string> reversePath = entity.Path.Split(",").Reverse();
-        foreach (var contentIdString in reversePath)
+        string[] contentIdStrings = entity.Path.Split(',');
+        for (int i = contentIdStrings.Length - 1; i >= 0; i--)
         {
-            var contentId = int.Parse(contentIdString, NumberStyles.Integer, CultureInfo.InvariantCulture);
+            var contentId = int.Parse(contentIdStrings[i], NumberStyles.Integer, CultureInfo.InvariantCulture);
             IEnumerable<IDomain> domains = _domainService.GetAssignedDomains(contentId, true);
             if (!domains.Any())
             {

src/Umbraco.Core/Extensions/StringExtensions.cs

@@ -57,16 +57,17 @@ static StringExtensions()
     /// <returns></returns>
     public static int[] GetIdsFromPathReversed(this string path)
     {
-        var nodeIds = path.Split(Constants.CharArrays.Comma, StringSplitOptions.RemoveEmptyEntries)
-            .Select(x =>
-                int.TryParse(x, NumberStyles.Integer, CultureInfo.InvariantCulture, out var output)
-                    ? Attempt<int>.Succeed(output)
-                    : Attempt<int>.Fail())
-            .Where(x => x.Success)
-            .Select(x => x.Result)
-            .Reverse()
-            .ToArray();
-        return nodeIds;
+        string[] pathSegments = path.Split(Constants.CharArrays.Comma, StringSplitOptions.RemoveEmptyEntries);
+        List<int> nodeIds = new(pathSegments.Length);
+        for (int i = pathSegments.Length - 1; i >= 0; i--)
+        {
+            if (int.TryParse(pathSegments[i], NumberStyles.Integer, CultureInfo.InvariantCulture, out int pathSegment))
+            {
+                nodeIds.Add(pathSegment);
+            }
+        }
+
+        return nodeIds.ToArray();
     }
 
     /// <summary>
@@ -79,7 +80,7 @@ public static int[] GetIdsFromPathReversed(this string path)
     public static string StripFileExtension(this string fileName)
     {
         // filenames cannot contain line breaks
-        if (fileName.Contains(Environment.NewLine) || fileName.Contains("\r") || fileName.Contains("\n"))
+        if (fileName.Contains('\n') || fileName.Contains('\r'))
         {
             return fileName;
         }
@@ -434,8 +435,7 @@ public static IList<string> ToDelimitedList(this string list, string delimiter =
     {
         var delimiters = new[] { delimiter };
         return !list.IsNullOrWhiteSpace()
-            ? list.Split(delimiters, StringSplitOptions.RemoveEmptyEntries)
-                .Select(i => i.Trim())
+            ? list.Split(delimiters, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)
                 .ToList()
             : new List<string>();
     }
@@ -617,7 +617,7 @@ public static bool InvariantEndsWith(this string compare, string compareTo) =>
         compare.EndsWith(compareTo, StringComparison.InvariantCultureIgnoreCase);
 
     public static bool InvariantContains(this string compare, string compareTo) =>
-        compare.IndexOf(compareTo, StringComparison.OrdinalIgnoreCase) >= 0;
+        compare.Contains(compareTo, StringComparison.OrdinalIgnoreCase);
 
     public static bool InvariantContains(this IEnumerable<string> compare, string compareTo) =>
         compare.Contains(compareTo, StringComparer.InvariantCultureIgnoreCase);

src/Umbraco.Core/Extensions/TreeEntityExtensions.cs

@@ -5,9 +5,20 @@ namespace Umbraco.Cms.Core.Extensions;
 
 public static class TreeEntityExtensions
 {
-    public static int[] AncestorIds(this ITreeEntity entity) => entity.Path
-        .Split(Constants.CharArrays.Comma)
-        .Select(item => int.Parse(item, CultureInfo.InvariantCulture))
-        .Take(new Range(Index.FromStart(1), Index.FromEnd(1)))
-        .ToArray();
+    public static int[] AncestorIds(this ITreeEntity entity)
+    {
+        string[] commaSeparatedValues = entity.Path.Split(Constants.CharArrays.Comma);
+        if (commaSeparatedValues.Length < 2)
+        {
+            return [];
+        }
+
+        int[] ancestorIds = new int[commaSeparatedValues.Length - 2];
+        for (int i = 1; i <= commaSeparatedValues.Length - 2; i++)
+        {
+            ancestorIds[i - 1] = int.Parse(commaSeparatedValues[i], CultureInfo.InvariantCulture);
+        }
+
+        return ancestorIds;
+    }
 }

src/Umbraco.Core/Models/PropertyTagsExtensions.cs

@@ -226,12 +226,12 @@ private static IEnumerable<string> GetTagsValue(this IProperty property, TagsSto
         switch (storageType)
         {
             case TagsStorageType.Csv:
-                return value.Split(new[] { delimiter }, StringSplitOptions.RemoveEmptyEntries).Select(x => x.Trim());
+                return value.Split([delimiter], StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
 
             case TagsStorageType.Json:
                 try
                 {
-                    return serializer.Deserialize<string[]>(value)?.Select(x => x.Trim()) ?? Enumerable.Empty<string>();
+                    return serializer.Deserialize<string[]>(value)?.Select(x => x.Trim()) ?? [];
                 }
                 catch (Exception)
                 {

src/Umbraco.Core/Services/LocalizedTextServiceExtensions.cs

@@ -52,7 +52,7 @@ public static string Localize(this ILocalizedTextService manager, string area, s
             return null;
         }
 
-        if (text.StartsWith("#") == false)
+        if (text.StartsWith('#') == false)
         {
             return text;
         }
@@ -64,7 +64,7 @@ public static string Localize(this ILocalizedTextService manager, string area, s
             return value;
         }
 
-        if (text.IndexOf('_') == -1)
+        if (!text.Contains('_'))
         {
             return text;
         }
@@ -77,7 +77,7 @@ public static string Localize(this ILocalizedTextService manager, string area, s
         }
 
         value = manager.Localize(areaAndKey[0], areaAndKey[1]);
-        return value.StartsWith("[") ? text : value;
+        return value.StartsWith('[') ? text : value;
     }
 
     /// <summary>

tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/TreeEntityExtensionsTests.cs

@@ -1,8 +1,7 @@
-using Moq;
+using Moq;
 using NUnit.Framework;
 using Umbraco.Cms.Core.Extensions;
 using Umbraco.Cms.Core.Models.Entities;
-using Range = System.Range;
 
 namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Extensions;
 

tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs

@@ -387,4 +387,12 @@ private static void TryIsFullPath(string path, bool expectedIsFull, bool expecte
             Assert.AreNotEqual(path, Path.GetFullPath(path));
         }
     }
+
+    [TestCase("1,2,3,4,5", "5,4,3,2,1")]
+    [TestCase("1,2,x,4,5", "5,4,2,1")]
+    public void GetIdsFromPathReversed(string input, string expected)
+    {
+        var ids = input.GetIdsFromPathReversed();
+        Assert.AreEqual(expected, string.Join(",", ids));
+    }
 }