From 30bbe57527d885cc89905903d992dae336d93da8 Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Sun, 11 Feb 2024 20:03:31 +0330 Subject: [PATCH 1/9] feat: add Parse method with parent parameter overload to HierarchyId abstraction --- .../HierarchyId.cs | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index e1bffb4d185..c93b9768d91 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics.CodeAnalysis; +using System.Text; using System.Text.Json.Serialization; using Microsoft.EntityFrameworkCore.Internal; using Microsoft.SqlServer.Types; @@ -25,7 +26,7 @@ public HierarchyId() } /// - /// Initializes a new instance of the class. Equivalent to . + /// Initializes a new instance of the class. Equivalent to . /// /// The string representation of the node. public HierarchyId(string value) @@ -33,6 +34,16 @@ public HierarchyId(string value) { } + /// + /// Initializes a new instance of the class. Equivalent to . + /// + /// The parent HierarchyId of node. + /// The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/". + public HierarchyId(HierarchyId parentHierarchyId, IReadOnlyList parentId) + : this(Parse(parentHierarchyId,parentId)) + { + } + /// /// Initializes a new instance of the class. /// @@ -63,6 +74,35 @@ public static HierarchyId GetRoot() public static HierarchyId? Parse(string? input) => (HierarchyId?)SqlHierarchyId.Parse(input); + /// + /// Converts the and of a node to a value. + /// + /// The parent HierarchyId of node. + /// The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/". + /// A value. + [return: NotNullIfNotNull(nameof(parentHierarchyId))] + [return: NotNullIfNotNull(nameof(parentId))] + public static HierarchyId? Parse(HierarchyId parentHierarchyId, IReadOnlyList parentId) + => GenerateHierarchyIdBaseOnParent(parentHierarchyId, parentId); + + //This Method can move to "SqlHierarchyId in Microsoft.SqlServer.Types", if we don't want put it in this abstraction. + private static HierarchyId GenerateHierarchyIdBaseOnParent(HierarchyId parent, IReadOnlyList parentId) + { + if (parentId.Count < 1) + return parent; + + var specificPath = new StringBuilder(parent.ToString()); + for (var i = 0; i < parentId.Count; i++) + { + specificPath.Append(parentId[i]); + if (i != parentId.Count - 1) + specificPath.Append('.'); + } + specificPath.Append('/'); + + return HierarchyId.Parse(specificPath.ToString()); + } + /// public virtual int CompareTo(HierarchyId? other) => _value.CompareTo((SqlHierarchyId)other); From af7a76c08bfb1233dfd9efeff26b161b80a99ba6 Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Sun, 11 Feb 2024 21:12:45 +0330 Subject: [PATCH 2/9] refactor: refactor and clean new parse overload --- src/EFCore.SqlServer.Abstractions/HierarchyId.cs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index c93b9768d91..23547105fd4 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -82,22 +82,20 @@ public static HierarchyId GetRoot() /// A value. [return: NotNullIfNotNull(nameof(parentHierarchyId))] [return: NotNullIfNotNull(nameof(parentId))] - public static HierarchyId? Parse(HierarchyId parentHierarchyId, IReadOnlyList parentId) - => GenerateHierarchyIdBaseOnParent(parentHierarchyId, parentId); + public static HierarchyId? Parse(HierarchyId parentHierarchyId , IReadOnlyList parentId) + => GenerateHierarchyIdBasedOnParent(parentHierarchyId, parentId); //This Method can move to "SqlHierarchyId in Microsoft.SqlServer.Types", if we don't want put it in this abstraction. - private static HierarchyId GenerateHierarchyIdBaseOnParent(HierarchyId parent, IReadOnlyList parentId) + private static HierarchyId GenerateHierarchyIdBasedOnParent(HierarchyId parent, IReadOnlyList parentId) { + if (parent is null) + return HierarchyId.GetRoot(); + if (parentId.Count < 1) return parent; var specificPath = new StringBuilder(parent.ToString()); - for (var i = 0; i < parentId.Count; i++) - { - specificPath.Append(parentId[i]); - if (i != parentId.Count - 1) - specificPath.Append('.'); - } + specificPath.Append(string.Join(".", parentId)); specificPath.Append('/'); return HierarchyId.Parse(specificPath.ToString()); From 8dc98c7c1e9be0d11358d309302014667f0f9db0 Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Sun, 11 Feb 2024 21:13:58 +0330 Subject: [PATCH 3/9] test: add test for parse overloads --- .../WrapperTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs b/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs index 475aacbbdab..a402f666b60 100644 --- a/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs +++ b/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs @@ -35,4 +35,18 @@ public void GetReparentedValue_returns_null_when_newRoot_is_null() [ConditionalFact] public void IsDescendantOf_returns_false_when_parent_is_null() => Assert.False(HierarchyId.Parse("/1/").IsDescendantOf(null)); + + [ConditionalFact] + public void Parse_Overloads_works() + { + var parent = HierarchyId.Parse("/1/"); + + Assert.Equal(HierarchyId.Parse(parent, parentId: [2]), HierarchyId.Parse("/1/2/")); + Assert.Equal(HierarchyId.Parse(parent, parentId: [2,1]), HierarchyId.Parse("/1/2.1/")); + Assert.Equal(HierarchyId.Parse(parent, parentId: []), HierarchyId.Parse("/1/")); + Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: [1]), HierarchyId.Parse("/1/")); + Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: []), HierarchyId.Parse("/")); + Assert.Equal(HierarchyId.Parse(null, parentId: []), HierarchyId.Parse("/")); + } + } From e74ea6d5365da965212bd9abfe25d477063023ce Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Sun, 11 Feb 2024 22:51:46 +0330 Subject: [PATCH 4/9] test: separate and refactor parse tests --- .../WrapperTests.cs | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs b/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs index a402f666b60..35e6845841b 100644 --- a/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs +++ b/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs @@ -37,16 +37,28 @@ public void IsDescendantOf_returns_false_when_parent_is_null() => Assert.False(HierarchyId.Parse("/1/").IsDescendantOf(null)); [ConditionalFact] - public void Parse_Overloads_works() - { - var parent = HierarchyId.Parse("/1/"); - - Assert.Equal(HierarchyId.Parse(parent, parentId: [2]), HierarchyId.Parse("/1/2/")); - Assert.Equal(HierarchyId.Parse(parent, parentId: [2,1]), HierarchyId.Parse("/1/2.1/")); - Assert.Equal(HierarchyId.Parse(parent, parentId: []), HierarchyId.Parse("/1/")); - Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: [1]), HierarchyId.Parse("/1/")); - Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: []), HierarchyId.Parse("/")); - Assert.Equal(HierarchyId.Parse(null, parentId: []), HierarchyId.Parse("/")); - } + public void Parse_overloads_works_when_parentId_is_simpleId() + => Assert.Equal(HierarchyId.Parse(_parent, parentId: [2]), HierarchyId.Parse("/1/2/")); + [ConditionalFact] + public void Parse_overloads_works_when_parentId_is_dottedString() + => Assert.Equal(HierarchyId.Parse(_parent, parentId: [2,1]), HierarchyId.Parse("/1/2.1/")); + + [ConditionalFact] + public void Parse_overloads_works_when_parentId_is_empty() + => Assert.Equal(HierarchyId.Parse(_parent, parentId: []), HierarchyId.Parse("/1/")); + + [ConditionalFact] + public void Parse_overloads_works_when_parentHierarchy_is_root_and_parentId_is_simple() + => Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: [1]), HierarchyId.Parse("/1/")); + + [ConditionalFact] + public void Parse_overloads_works_when_parentHierarchy_is_root_and_parentId_is_empty() + => Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: []), HierarchyId.Parse("/")); + + [ConditionalFact] + public void Parse_overloads_works_when_parentHierarchy_is_null_and_parentId_is_empty() + => Assert.Equal(HierarchyId.Parse(null, parentId: []), HierarchyId.Parse("/")); + + private readonly HierarchyId _parent = HierarchyId.Parse("/1/"); } From 595a85ea654df6740f1d8e99424b45686a5a860b Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Fri, 16 Feb 2024 19:01:02 +0330 Subject: [PATCH 5/9] fix: #32943-resolve [review comment](https://github.com/dotnet/efcore/pull/33062#discussion_r1491452316) --- src/EFCore.SqlServer.Abstractions/HierarchyId.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index 23547105fd4..253d0995877 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -89,10 +89,14 @@ public static HierarchyId GetRoot() private static HierarchyId GenerateHierarchyIdBasedOnParent(HierarchyId parent, IReadOnlyList parentId) { if (parent is null) + { return HierarchyId.GetRoot(); + } if (parentId.Count < 1) + { return parent; + } var specificPath = new StringBuilder(parent.ToString()); specificPath.Append(string.Join(".", parentId)); From 44a28088134bba45cf3ae6e827b34e57d586809c Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Fri, 16 Feb 2024 19:01:02 +0330 Subject: [PATCH 6/9] fix: #32943 - resolve [review comment](https://github.com/dotnet/efcore/pull/33062#discussion_r1491452316) --- src/EFCore.SqlServer.Abstractions/HierarchyId.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index 23547105fd4..253d0995877 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -89,10 +89,14 @@ public static HierarchyId GetRoot() private static HierarchyId GenerateHierarchyIdBasedOnParent(HierarchyId parent, IReadOnlyList parentId) { if (parent is null) + { return HierarchyId.GetRoot(); + } if (parentId.Count < 1) + { return parent; + } var specificPath = new StringBuilder(parent.ToString()); specificPath.Append(string.Join(".", parentId)); From d9264064cf8b326a0f4854cb3ce091edab55ccae Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Fri, 16 Feb 2024 19:47:53 +0330 Subject: [PATCH 7/9] fix: #32943 - resolve [review comment](https://github.com/dotnet/efcore/pull/33062#discussion_r1491451862) --- src/EFCore.SqlServer.Abstractions/HierarchyId.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index 253d0995877..4e286841209 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -80,8 +80,6 @@ public static HierarchyId GetRoot() /// The parent HierarchyId of node. /// The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/". /// A value. - [return: NotNullIfNotNull(nameof(parentHierarchyId))] - [return: NotNullIfNotNull(nameof(parentId))] public static HierarchyId? Parse(HierarchyId parentHierarchyId , IReadOnlyList parentId) => GenerateHierarchyIdBasedOnParent(parentHierarchyId, parentId); From b214f1cc310d0a7491ce441b2c98d31c71ff788a Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Fri, 16 Feb 2024 20:16:11 +0330 Subject: [PATCH 8/9] refactor: #32943 - replace IReadOnlyList to params for easier use --- src/EFCore.SqlServer.Abstractions/HierarchyId.cs | 16 +++------------- .../WrapperTests.cs | 12 ++++++------ 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index 4e286841209..e505d269979 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -34,16 +34,6 @@ public HierarchyId(string value) { } - /// - /// Initializes a new instance of the class. Equivalent to . - /// - /// The parent HierarchyId of node. - /// The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/". - public HierarchyId(HierarchyId parentHierarchyId, IReadOnlyList parentId) - : this(Parse(parentHierarchyId,parentId)) - { - } - /// /// Initializes a new instance of the class. /// @@ -80,18 +70,18 @@ public static HierarchyId GetRoot() /// The parent HierarchyId of node. /// The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/". /// A value. - public static HierarchyId? Parse(HierarchyId parentHierarchyId , IReadOnlyList parentId) + public static HierarchyId? Parse(HierarchyId parentHierarchyId , params int[] parentId) => GenerateHierarchyIdBasedOnParent(parentHierarchyId, parentId); //This Method can move to "SqlHierarchyId in Microsoft.SqlServer.Types", if we don't want put it in this abstraction. - private static HierarchyId GenerateHierarchyIdBasedOnParent(HierarchyId parent, IReadOnlyList parentId) + private static HierarchyId GenerateHierarchyIdBasedOnParent(HierarchyId parent, params int[] parentId) { if (parent is null) { return HierarchyId.GetRoot(); } - if (parentId.Count < 1) + if (parentId.Length < 1) { return parent; } diff --git a/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs b/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs index 35e6845841b..02672cd176d 100644 --- a/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs +++ b/test/EFCore.SqlServer.HierarchyId.Tests/WrapperTests.cs @@ -38,27 +38,27 @@ public void IsDescendantOf_returns_false_when_parent_is_null() [ConditionalFact] public void Parse_overloads_works_when_parentId_is_simpleId() - => Assert.Equal(HierarchyId.Parse(_parent, parentId: [2]), HierarchyId.Parse("/1/2/")); + => Assert.Equal(HierarchyId.Parse(_parent, 2), HierarchyId.Parse("/1/2/")); [ConditionalFact] public void Parse_overloads_works_when_parentId_is_dottedString() - => Assert.Equal(HierarchyId.Parse(_parent, parentId: [2,1]), HierarchyId.Parse("/1/2.1/")); + => Assert.Equal(HierarchyId.Parse(_parent, 2,1), HierarchyId.Parse("/1/2.1/")); [ConditionalFact] public void Parse_overloads_works_when_parentId_is_empty() - => Assert.Equal(HierarchyId.Parse(_parent, parentId: []), HierarchyId.Parse("/1/")); + => Assert.Equal(HierarchyId.Parse(_parent), HierarchyId.Parse("/1/")); [ConditionalFact] public void Parse_overloads_works_when_parentHierarchy_is_root_and_parentId_is_simple() - => Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: [1]), HierarchyId.Parse("/1/")); + => Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(),1), HierarchyId.Parse("/1/")); [ConditionalFact] public void Parse_overloads_works_when_parentHierarchy_is_root_and_parentId_is_empty() - => Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot(), parentId: []), HierarchyId.Parse("/")); + => Assert.Equal(HierarchyId.Parse(HierarchyId.GetRoot()), HierarchyId.Parse("/")); [ConditionalFact] public void Parse_overloads_works_when_parentHierarchy_is_null_and_parentId_is_empty() - => Assert.Equal(HierarchyId.Parse(null, parentId: []), HierarchyId.Parse("/")); + => Assert.Equal(HierarchyId.Parse(null,[]), HierarchyId.Parse("/")); private readonly HierarchyId _parent = HierarchyId.Parse("/1/"); } From a6c9ae9e51d9ad00be250d2087924eaa2cd9ed2a Mon Sep 17 00:00:00 2001 From: "rezakazemi890@gmail.com" Date: Wed, 28 Feb 2024 12:36:41 +0330 Subject: [PATCH 9/9] fix: #32943 - resolve [review comment](https://github.com/dotnet/efcore/pull/33062#discussion_r1505577130) --- src/EFCore.SqlServer.Abstractions/HierarchyId.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs index e505d269979..04dc58d058d 100644 --- a/src/EFCore.SqlServer.Abstractions/HierarchyId.cs +++ b/src/EFCore.SqlServer.Abstractions/HierarchyId.cs @@ -70,7 +70,7 @@ public static HierarchyId GetRoot() /// The parent HierarchyId of node. /// The parent Id of current node. It can be more than one element if want have path like: "/1/2/3.1/", otherwise one element for have path like: "/1/2/3/". /// A value. - public static HierarchyId? Parse(HierarchyId parentHierarchyId , params int[] parentId) + public static HierarchyId Parse(HierarchyId parentHierarchyId , params int[] parentId) => GenerateHierarchyIdBasedOnParent(parentHierarchyId, parentId); //This Method can move to "SqlHierarchyId in Microsoft.SqlServer.Types", if we don't want put it in this abstraction.