MarimerLLC / csla

A home for your business logic in any .NET application.
https://cslanet.com
MIT License
1.27k stars 406 forks source link

AmbiguousMatchException DataPortal.Fetch #2109

Closed coder-rr closed 3 years ago

coder-rr commented 3 years ago

Describe the bug Upgrading to 5.4.1 has a breaking change. Can't have a DataPortal Interface method parameter without decorating method with [Fetch] attribute.

private void DataPortal_Fetch(ICriteriaBase criteria)
private void DataPortal_Fetch(IEnumerable<string> criteria)

Also happens using the new modifier on DataPortal_Execute. protected new async Task DataPortal_Execute

Version and Platform CSLA version: 5.4.1 OS: Windows, Linux, iOS, Android, etc. Platform: WinForms, WPF, ASP.NET Core, MVC, Xamarin, etc.

Code that Fails Sample app runs fine using 5.4.0. You will need to update csla to 5.4.1 to cause the error in the sample app. https://github.com/coder-rr/CslaDpTest

Stack Trace or Exception Detail Exception Message

CslaDpTest.CarList.[Fetch](String[]). Matches: 
CslaDpTest.CarList[Void DataPortal_Fetch(System.Collections.Generic.IEnumerable`1[System.String])],
Csla.ReadOnlyListBase`2[[CslaDpTest.CarList, CslaDpTest, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],
[CslaDpTest.CarInfo, CslaDpTest, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]][Void DataPortal_Fetch(System.Object)]

Stack Trace

at Csla.Reflection.ServiceProviderMethodCaller.FindDataPortalMethod[T](Type targetType, Object[] criteria, Boolean throwOnError)
at Csla.Server.DataPortal.<Fetch>d__11.MoveNext()
rockfordlhotka commented 3 years ago

Thank you for reporting this issue, I'll add it to the backlog.

For your future info and planning, the idea is ultimately to require the use of the operation attributes and entirely eliminate the hard-coded method names (like DataPortal_Fetch) entirely. That process will start in CSLA 6.

rockfordlhotka commented 3 years ago

It should be possible to replicate this issue by adding a test to ServiceProviderMethodCallerTests, and then work from there to solve the issue.

coder-rr commented 3 years ago

https://github.com/MarimerLLC/csla/issues/1994 ServiceProviderMethodCaller cannot resolve factory methods in base class.

Looks like this caused the issue https://github.com/MarimerLLC/csla/pull/2020/commits/0aadede5fc5c658870bedec38a2451fd7777b8ee

And this fixed it - #1994 Only apply change to factory methods, not all methods https://github.com/MarimerLLC/csla/commit/751fb367994c14320702086983a1ebb69ec10e9e

rockfordlhotka commented 3 years ago

Oh, so this issue is no longer valid?

coder-rr commented 3 years ago

It has been fixed in Main, but it is broken in 5.4.1

rockfordlhotka commented 3 years ago

I'm not sure it is fixed in either place, as #1994 was closed/merged in 5.4.1 - so it clearly didn't fix the issue.

rockfordlhotka commented 3 years ago

@coder-rr I am unable to replicate this issue in unit tests.

Here's my test, can you look and see if I'm missing the intent of your issue?

coder-rr commented 3 years ago

I made a change to the test. The issue is when using ReadOnlyListBase and DataPortal_Fetch method. Might have something to do with the fact that ReadOnlyListBase has a virtual DataPortal_Fetch?

https://github.com/coder-rr/Csal/commit/23286cf11ae5d53ce3a8788ae3f53094a5a78bc7#diff-b2d2252bec75ae1e2c848994076a9c993ad70c548ae03e35b57cc45b4db3ea67

The AmbiguousMatchException was fixed with this commit. It is not in 5.4.1 https://github.com/MarimerLLC/csla/commit/751fb367994c14320702086983a1ebb69ec10e9e#diff-18da7607bda22803d193d7e5b78e45ca122e63feaf81d406662cd4d7dc872ccb

rockfordlhotka commented 3 years ago

Oh, that could be. Are you using the Fetch attribute?

The DP_XYZ methods are going away in CSLA 6, so that might help in the future šŸ˜‰

This is good, thank you for the help - hopefully I'll have time to get back to this later today and can work on a fix in 5.4.x.

coder-rr commented 3 years ago

Thanks!

Some of our code still uses DP_XYZ methods. We are in the process of adding the DP operation attribute to our dp methods.

rockfordlhotka commented 3 years ago

There's an analyzer that should help just do that project-wide.

rockfordlhotka commented 3 years ago

@coder-rr I'm still struggling to replicate this.

I created a console app that uses the data portal in the way (I think) you are using it. Here's the complete code that does not fail in the 5.4.2 code:

using Csla;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;

namespace ConsoleApp4
{
  class Program
  {
    static async Task Main(string[] args)
    {
      var criteria = new Issue2109.MyCriteria { Name = "1" };
      var obj = await DataPortal.FetchAsync<Issue2109>(criteria);
      Console.WriteLine(obj.Name);
      var values = new string[] { "a" };
      obj = await DataPortal.FetchAsync<Issue2109>(values);
      Console.WriteLine(obj.Name);
    }
  }

  [Serializable]
  public class Issue2109 : BusinessBase<Issue2109>
  {
    public static readonly PropertyInfo<string> NameProperty = RegisterProperty<string>(nameof(Name));
    public string Name
    {
      get => GetProperty(NameProperty);
      set => SetProperty(NameProperty, value);
    }

    private void DataPortal_Fetch(ICriteriaBase criteria)
    {
      using (BypassPropertyChecks)
      {
        Name = criteria.Name;
      }
    }

    private void DataPortal_Fetch(IEnumerable<string> criteria)
    {
      using (BypassPropertyChecks)
      {
        Name = criteria.First();
      }
    }

    public interface ICriteriaBase
    { 
      string Name { get; set; }
    }

    [Serializable]
    public class MyCriteria : ICriteriaBase
    {
      public string Name { get; set; }
    }
  }
}
coder-rr commented 3 years ago

@rockfordlhotka The following code is breaking in 5.4.1

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Csla;

namespace ConsoleApp4
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var criteria = new Issue2109List.MyCriteria { Name = "1" };
            var obj = await DataPortal.FetchAsync<Issue2109List>(criteria);
            Console.WriteLine(obj[0].Name);
            var values = new string[] { "a" };
            obj = await DataPortal.FetchAsync<Issue2109List>(values);
            Console.WriteLine(obj[0].Name);
        }
    }

    [Serializable]
    public class Issue2109List : ReadOnlyListBase<Issue2109List, Issue2109>
    {
        private void DataPortal_Fetch(ICriteriaBase criteria)
        {
            Add(Issue2109.Get(criteria.Name));
        }

        private void DataPortal_Fetch(IEnumerable<string> criteria)
        {
            Add(Issue2109.Get(criteria.First()));
        }

        public interface ICriteriaBase
        {
            string Name { get; set; }
        }

        [Serializable]
        public class MyCriteria : ICriteriaBase
        {
            public string Name { get; set; }
        }
    }

    [Serializable]
    public class Issue2109 : ReadOnlyBase<Issue2109>
    {
        public static readonly PropertyInfo<string> NameProperty = RegisterProperty<string>(nameof(Name));

        public string Name
        {
            get => GetProperty(NameProperty);
            set => LoadProperty(NameProperty, value);
        }

        public static Issue2109 Get(string name) => DataPortal.Fetch<Issue2109>(name);

        private void DataPortal_Fetch(string name)
        {
            Name = name;
        }
    }
}
rockfordlhotka commented 3 years ago

@coder-rr sorry for not getting to this for a while.

The problem exists in a number of base types:

All of these have a protected DataPortal_Fetch(object) method declaration, which is conflicting with the string[] overload.

coder-rr commented 3 years ago

Thanks for looking into this. Our next application release will contain refactored business objects to include the data portal operation attribute.

On Mon, Apr 5, 2021 at 1:30 PM Rockford Lhotka @.***> wrote:

@coder-rr https://github.com/coder-rr sorry for not getting to this for a while. I believe I've updated the code in the test https://github.com/rockfordlhotka/csla/blob/26bfb1818410465eb45bb5a906ae0fb230867872/Source/csla.netcore.test/DataPortal/ServiceProviderMethodCallerTests.cs#L253 to reflect your scenario, and it is still not failing.

ā€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/MarimerLLC/csla/issues/2109#issuecomment-813561564, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWVTIGJHFTL4MVFMGM2MIDTHH6VPANCNFSM4XZAWDWQ .

rockfordlhotka commented 3 years ago

Regardless, this was a bug that needed fixing, and I'm glad you brought it up and helped figure it out! Thank you.