zzzprojects / EntityFramework-Classic

Entity Framework Classic is a supported version of the latest EF6 codebase. It supports .NET Framework and .NET Core and overcomes some EF limitations by adding tons of must-haves built-in features.
https://entityframework-classic.net
Other
103 stars 27 forks source link

HierarchyId.GetAncestor method raise an unexpected exception #63

Closed rmaenk closed 2 years ago

rmaenk commented 3 years ago

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

JonathanMagnan commented 3 years ago

Hello @rmaenk ,

Thank you for reporting,

Sure, we will look at this.

JonathanMagnan commented 3 years ago

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 commented 3 years ago

Hello @JonathanMagnan ,

Thank you for informing.

Best Regards, Roman

JonathanMagnan commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

Hello @JonathanMagnan ,

Sorry for delay,

The issue is fixed in v.7.1.46.

Thanks a lot! Roman