SkywardApps / popcorn

Popcorn is a .Net Middleware for your RESTful API that allows your consumers to request exactly as much or as little as they need, with no effort from you.
https://skywardapps.github.io/popcorn/
MIT License
59 stars 19 forks source link

Compiler picks the wrong overload of `Translate` #13

Closed jmathew closed 6 years ago

jmathew commented 7 years ago

Related to https://stackoverflow.com/q/46819033/730326

Steps to Reproduce

Given a model like so:

public class DataModel {
    public List<Person> Persons { get; set; }
}

public class Person {
    public Guid Id { get; set; }
    public List<Pet> Pets {get; set;}

    public List<Pet> GetLivingPets() {
       // Do some computation to get the pets that are alive
    }
}

public class Pet {
    public Guid Id { get; set; }
    public string Name { get; set; }
    public bool Alive { get; set; }
}

public PetProjection {
    public Guid? Id { get; set; }
    public string Name { get; set; } 
}

public PersonProjection {
    public Guid? Id { get; set; }
    public List<PetProjection> LivingPets {get; set;}
}

And a mapping like so:

popcornConfig
.Map<Person, PersonProjection>(config: (personConfig) =>
{

     personConfig.Translate(fp => fp.LivingPets, f => f.GetLivingPets());

})
.MapEntityFramework<Pet, PetProjection, DataModel>(dbContextOptionsBuilder);

Expected Behavior

No error. Compiles and provides expansion for the Pets. (ie persons?include=[Name, LivingPets[Name]] returns only the pets names not the whole object)

Actual Behavior

On the .Translate line.

'Dictionary<string, object>' does not contain a definition for 'GetLivingPets' and no extension method 'GetLivingPets' accepting a first argument of type 'Dictionary<string, object>' could be found (are you missing a using directive or an assembly reference?)   

Work around

If I change the lambda signature to personConfig.Translate(fp => fp.LivingPets, (f, dict) => f.GetLivingPets()); it works just fine.

Alternatively, if I rename GetLivingPets() to LivingPets() I can remove the translation and it all works.

undiwahn commented 7 years ago

I think the fix for this will unfortunately be to rebrand a subset of the 'Translate' overloads to 'TranslateWithContext' or otherwise differentiating the different versions by name. We're relying on the compiler to be smarter than it is possible to be in this situation, and realistically not a lot is gained by simply overloading the same method so many times.

Eightgate commented 7 years ago

Claimed