Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HierarchyId.GetAncestor method raise an unexpected exception #63

Closed
rmaenk opened this issue Apr 30, 2021 · 7 comments
Closed

HierarchyId.GetAncestor method raise an unexpected exception #63

rmaenk opened this issue Apr 30, 2021 · 7 comments
Assignees

Comments

@rmaenk
Copy link

rmaenk commented Apr 30, 2021

Hello,
I am using version 7.1.43 of the library.
This code produces an exception:

    [TestMethod]
    public void HidGetAncestorTest() {
        HierarchyId hid = HierarchyId.GetRoot().GetDescendant(null, null);
        HierarchyId parent = hid.GetAncestor(1);
        Assert.AreEqual(HierarchyId.GetRoot().ToString(), parent.ToString());
    }

Error:

Test method UnitTests.DynamicCollectionTests.HidGetAncestorTest threw exception: 
System.ArgumentException: The input string '//' is not a valid string representation of a HierarchyId node.
Parameter name: hierarchyId
    в System.Data.Entity.Hierarchy.HierarchyId..ctor(String hierarchyId)
   в System.Data.Entity.Hierarchy.HierarchyId.GetAncestor(Int32 n)

The reason is a wrong implementation of GetAncestor method in this library version:

    /// <summary>
    ///     Returns a hierarchyid representing the nth ancestor of this.
    /// </summary>
    /// <returns>A hierarchyid representing the nth ancestor of this.</returns>
    /// <param name="n">n</param>
    public HierarchyId GetAncestor(int n) => this._nodes == null || (int) this.GetLevel() < n ? new HierarchyId((string) null) : new HierarchyId("/" + string.Join("/", ((IEnumerable<int[]>) this._nodes).Take<int[]>((int) this.GetLevel() - n).Select<int[], string>(new Func<int[], string>(HierarchyId.IntArrayToStirng))) + "/");

EntityFramework 6.4.0 has this fixed:

    /// <summary>
    ///     Returns a hierarchyid representing the nth ancestor of this.
    /// </summary>
    /// <returns>A hierarchyid representing the nth ancestor of this.</returns>
    /// <param name="n">n</param>
    public HierarchyId GetAncestor(int n)
    {
      if (this._nodes == null || (int) this.GetLevel() < n)
        return new HierarchyId((string) null);
      return (int) this.GetLevel() == n ? new HierarchyId("/") : new HierarchyId("/" + string.Join("/", ((IEnumerable<int[]>) this._nodes).Take<int[]>((int) this.GetLevel() - n).Select<int[], string>(new Func<int[], string>(HierarchyId.IntArrayToStirng))) + "/");
    }

this is the fix:

(int) this.GetLevel() == n ? new HierarchyId("/") : 

Merging EF 6.4 to Classic is desired to fix this, but may take significant time.
Could you please make a hotfix in the library?

It would be very helpful.

Thanks,
Roman

@rmaenk rmaenk changed the title HierarchyId.GetAncestor method raise an unexpected exception HierarchyId.GetAncestor method raise a unexpected exception Apr 30, 2021
@rmaenk rmaenk changed the title HierarchyId.GetAncestor method raise a unexpected exception HierarchyId.GetAncestor method raise an unexpected exception Apr 30, 2021
@JonathanMagnan JonathanMagnan self-assigned this Apr 30, 2021
@JonathanMagnan
Copy link
Member

Hello @rmaenk ,

Thank you for reporting,

Sure, we will look at this.

@JonathanMagnan
Copy link
Member

Hello @rmaenk ,

We didn't forget you, merged recent change currently takes us more time than expected.

However, we are still working on it.

If everything goes as expected, a new version should be released next Wednesday.

Best Regards,

Jon

@rmaenk
Copy link
Author

rmaenk commented May 6, 2021

Hello @JonathanMagnan ,

Thank you for informing.

Best Regards,
Roman

@JonathanMagnan
Copy link
Member

Hello @rmaenk ,

The v7.1.46 has been released.

We tried to merge all fixes since the last time but unfortunately, there is currently too much. So we choose to finally just do it by small chunk.

We included the fix in this version, could you try it and let us know if everything is working correctly?

Best Regards,

Jon

@JonathanMagnan
Copy link
Member

Hello @rmaenk .

Since our last conversation, we haven't heard from you!

Did you get the chance to try v7.1.46?

Let us know if everything is now working correctly.

Best regards,

Jon

@JonathanMagnan
Copy link
Member

Hello @rmaenk ,

A simple reminder that we are here to assist you!

Feel free to contact us once you try the v7.1.46.

Best regards,

Jon

@rmaenk
Copy link
Author

rmaenk commented Jul 23, 2021

Hello @JonathanMagnan ,

Sorry for delay,

The issue is fixed in v.7.1.46.

Thanks a lot!
Roman

@rmaenk rmaenk closed this as completed Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants