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

Cannot use [MapDerivedType] with Map(IQueryable) #409

Closed
x2764tech opened this issue May 3, 2023 · 6 comments · Fixed by #412
Closed

Cannot use [MapDerivedType] with Map(IQueryable) #409

x2764tech opened this issue May 3, 2023 · 6 comments · Fixed by #412
Assignees
Labels
bug Something isn't working released on @next
Milestone

Comments

@x2764tech
Copy link

Describe the bug
When using [MapDerivedType] and Map(IQuerable) in the same class, the generated code includes a switch statement in the Queryable.Select lambda

To Reproduce
Steps to reproduce the behavior:

  1. Declare a type Hierarchy with at least one base class and one derived class - let's call them Parent and Child
  2. Declare corresponding DTO types for those classes - ParentDto and ChildDto
  3. Declare a static Mapper class and add a method with the following Signature : [MapDerivedType<Child, ChildDto>] public static partial ParentDto ToDto(Parent model);
  4. Add the following method to the Mapper: public partial static IQueryable<ParentDto> ProjectToDto(this IQueryable<Parent> query);
  5. The generated code will contain a switch statement in the generated lambda, which is not valid for Queryable expressions

Expected behavior
The generated code should compile, and ideally map the hierachy correctly

Code snippets

public abstract class Parent {}
public class Child : Parent {}

public abstract class ParentDto {}
public class ChildDto: ParentDto {} //the inheritance here is unimportant, but is relevant for my use case

[Mapper]
public static partial class ParentChildMapper
{
    [MappDerivedType<Child, ChildDto>()]
    public static partial ParentDto ToDto(this Parent parent);

    public static partial IQuerable<ParentDto> ProjectToDto(this IQueryable<Parent> query);
}

Environment (please complete the following information):

  • Mapperly Version: main branch (unreleased)
  • .NET Version: 7.0.203
  • Target Framework: .net7.0
  • IDE: Rider 2023.1.1
  • OS:

Additional context

I know this is pre-release, and I'm not using it in production, but it would be a nice use-case to be fulfilled.

@latonz latonz added the bug Something isn't working label May 3, 2023
@latonz
Copy link
Contributor

latonz commented May 3, 2023

Thank you for the bug report 😊
Do you have an idea how the code could look like for such a mapping? How would you write it manually?
If it does not work, Mapperly should at least ignore such mappings for queryable projections and probably diagnostic.

@x2764tech
Copy link
Author

You have to use the ternary operator and is:

Select(object => object is Child ?  new ChildDto() : object is OtherChild ? new OtherChildDto() : default  )

note: needs to use more generic projection than new T()
note: this seems like a case where code generation wins, as a complex hierarchy is going to get complicated pretty quickly.
note: I'd also expect the tests to start from the bottom of the inheritance tree and work upwards - EF might do the right thing, but other implementations might not have a "discriminator" type column

@onionhammer
Copy link

Hmm how would " is " translate to SQL (or anything else)? This case the user would likely have to manually write an expression<> mapper function 99% of the time.

@x2764tech
Copy link
Author

@onionhammer It works - EF core will select the required columns and then do the "is" calculation client side. It may lead to over fetching, and maybe left joins (which might be avoided using subqueries with EF7)

@latonz
Copy link
Contributor

latonz commented May 8, 2023

This should now be fixed on main.

@x2764tech
Copy link
Author

That's amazing. Thank you 🙏

@latonz latonz added this to the v2.9.0 milestone Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released on @next
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants