Supply additional parameters to the mapping method about mapperly

[darlov]

From my perspective every parameters should have the attribute to identify the destination property for assign to. It's will avoid mistakes when properties will renamed and it's add ability to rename all mappings without manual work for rename every parameters.
Look's like this:

[Mapper]
public partial class CarMapper
{
    public partial Car Map(CarDto dto, [MapTo(nameof(Car.TenantId))] string tenantId);
}

[AttributeUsage(AttributeTargets.Parameter)]
public sealed class MapTo : Attribute
{
  public MapTo(string name)
  {
    Name = name;
  }
  
  public string Name { get; }
}

[MisinformedDNA]

This sounds like a terrible idea to do using AutoMapper or Mapperly. Writing a few manual mappings for complex situations seems like more maintainable for the codebase and for this library.

That's the same argument against Mapperly and AutoMapper in general. Whether it is terrible is subjective and developers can choose to use it or not.

[latonz]

I like this idea. However, it is not that easy to implement. It may take some time until Mapperly supports this.

[DerMave]

@latonz I haven't really gotten around to it yet. I made some unit tests and looked at the relevant bits and pieces. It seems I have underestimated the scope of this change. I am still interested in going forward with it, but I have to find some free time.

[latonz]

@Fube I don't think flattening/unflattening would be the right approach for this, it could work as a workaround though.
IMO the way to go would either be providing parameters directly to the mapping methods which are then mapped by name to properties of the target object or to add a secondary source objects which are also considered as mapping sources.

[TimothyMakkison]

I'm torn between two ideas for this feature and unsure which one would be better:

Top Level

• Additional parameters are only used on the immediate mapping, generated NewInstanceObjectMemberMapping will not pass parameters down.

// this works
public partial DogDto ToDogDto(Dog src, int id)
{
    return new DogDto(src.Name, id);
}

// will not pass parameter down
public partial CatOwnerDto ToCatOwnerDto(CatOwner src, int id)
{
    var target = new CatOwnerDto()
    {
        // id will not be passed
        Cat = MapToCatDto(src.Cat);
    }
    return target;
}

• If a user defined or user implemented method is found with similar parameters it will be used.

// User defined mapping (can also be user implemented)
public partial CatDto ToCatDto(Cat src, int id) ...
// public CatDto ToCatDto(Cat src, int id) ...

public partial CatOwnerDto ToCatOwnerDto(CatOwner src, int id)
{
    var target = new CatOwnerDto()
    {
        // id will be passed
        Cat = ToCatDto(src.Cat, id);
    }
    return target;
}

Pros

• Faster to generate
• Simple
• Top level is similar to other mapperly features

Cons

• Cumbersome
  • Using an extra parameter for a nested property forces you to add a user defined mappings for each step
    • Could use [MapProperty] (won't work with init/required)

Nested

• Additional parameters can be used for all levels of the mapping.
  • The mapper is smart enough to determine if a new method needs an extra parameter, even if a child of the new method uses it. (Its also recursion safe)
• Methods created by the mapping that use extra parameters are scoped to the current user defined mapping to avoid resolving/config issues.
• Can use other user implemented or user defined methods.

public partial CatOwnerDto ToCatOwnerDto(CatOwner src, int id)
{
    var target = new CatOwnerDto()
    {
        // below method was created by Mapperly, no user defined method required
        // if the method doesn't need `id`, then it won't be passed
        Cat = MapToCatDto(src.Cat, id);
    }
    return target;
}

Pros

• Easier to use
• In the future logic can be adapted to support nested ignores and nested init/required mapping and more.
• Works with nested init/required

Cons

• Slower generation
• Potentially confusing
• Will likely require nested mapping and nested ignores to be added

I have a working POC for the nested logic mapper see #550. It can be adapted to do top level mappings

[Herdo]

This is currently the blocking issue for me to switch from AutoMapper to Mapperly.

I'm receiving requests that are mapped to an internal command and require additional properties from the HttpContext.

[Herdo]

@TimothyMakkison I think most of the listed questions derive from the fact that you approach it in an implicit way.

Wouldn't most of these questions vanish if we followed an explicit approach by introducing an additional attribute:

[MapAdditionalValueToProperty("id", "ChipId")]
public partial DogDto MapDogToDto(Dog dog, Guid id);

Attribute name could be whatever, just some thoughts.

• MapAddition
• MapExtra
• MapExternal
• ...

[Fube]

@DerMave
Unfortunately not, but feel free to takeover.
I investigated the issue, but did not have time to come up with a solution.

As far as I can tell, the TypeMapping has to be changed to account for more than one source.

[latonz]

I'm busy right now and I'm afraid I won't find time in the upcoming days to look into this 😞, but I'll look into it in the upcoming weeks. In the meantime I'd like to focus on getting v2.9.0 released 😊

[Tvde1]

This sounds like a terrible idea to do using AutoMapper or Mapperly. Writing a few manual mappings for complex situations seems like more maintainable for the codebase and for this library.

[Fube]

@latonz
Do you have an architectural approach in mind for this?

Would you rework TypeMapping itself to include more than one ITypeSymbol for the source or would you build another type on top of it, something like a MultiSourceTypeMapping which would hold multiple TypeMappings that point to the same target type.

I would love to contribute, but don't want to go against the vision you might have for this 🙂

[latonz]

@Fube Sorry, I missed the notification of your comment.
I didn't have the time to think about a concrete architecture yet. Are all arguments considered as mapping sources? Or are the additional arguments only sources for properties with the same name? On a first glance I think if all arguments are considered as mapping sources, the TypeMapping needs to be refactored. Otherwise it seems to be limited to the PropertyMapping and it's related classes. Tbd is also how to handle it, if another mapping depends on a user defined mapping with additional arguments.

Feel free to submit an architectural proposal to this issue.

[DerMave]

I would like to work on this feature as I have an usecase for it. Have you begun working on this? @latonz @Fube

[latonz]

@DerMave I didn't have time yet to work on this. Let me know if you start working on this, then I'd assign the issue to you.
However, before you start with an actual implementation, I think it would be useful to discuss a design proposal on how this feature should work exactly and which parts of Mapperly need adjustments.

[akoken]

+1 for this feature. I think this is very common use case. As @cyril265 mentioned above, we also obtain tenantId, userId etc. from HttpContext.

[Fube]

Maybe we can use the flattening / unflattening logic to implement this
We could make an intermediary type to nest the arguments and then flatten it into the target type

Though it is a bit hacky, if it is possible, it would be the simplest way to implement this

Otherwise, we're looking at a change in IMapping which would propagate throughout the rest of the code

Not sure if my suggestion makes sense though, @latonz is it plausible?

[TimothyMakkison]

I have a working POC for this feature, I've opted to change IMapping and add MethodParameter[] Parameters { get; } (probably shouldn't be using MethodParameter)

I have some questions/concerns about breaking future features.

• How should method resolving work?

  • For void method mappings should the target member always be the second non type/IRefernceHandler parameter
  • Derived type mapping uses Type as a discriminator, unlike IReferenceHandler it doesn't use an attribute label. Could this cause issues while parsing the method?
  • Might cause future issues with #276

• Where are additional parameter mappings allowed?

  • Stay in the first method scope
  • Pass all the parameters to all child methods
  • Used in methods when needed (harder but possible)

• Can IQueryable have parameters?

• Should multi param user implemented mapping be allowed. How should the resolution be handled? ie match by type and name.

  • When trying to use a user mapping should Mapperly attempt to resolve the parameters by finding matching members? ie ToDogDto(src.Dog, src.DogOwner, id)

• I'm currently using the name of the variable to determine if it can map to a member. This feels natural but is a bit of an anti pattern, plus Mapperly doesn't do this for the source type.

[TimothyMakkison]

@Herdo you're right most of my problems are self inflicted 😄
I almost mentioned an explicit attribute but decided to try implementing implicit parameters first. It feels much more idiomatic.

[MisinformedDNA]

Ideally, this would work without any additional attributes or modifications:

public record Person(string FirstName, string LastName);
public record Employee(string FirstName, string LastName, string EmployeeId);

public static partial Employee ToEmployee(this Person person, string EmployeeId);

Workaround

public record Employee(string FirstName, string LastName, string EmployeeId)
{
    public string EmployeeId { get; init; } = default!;
}

public static Employee ToEmployee(this Person person, string employeeId)
{
    return person.ToEmployee() with { EmployeeId = EmployeeId };
}

[MapperIgnoreSource(nameof(Employee.EmployeeId))]
private static partial Employee ToEmployee(this Person person);

[fizmhd]

little late to the discussion, it would be nice to have this feature,
any update on this one ?

[latonz]

No updates, feel free to contribute.

I think before an implementation, several things need to be thought of and decided:

• Are mapping methods with additional parameters still resolved by Mapperly in transitive mappings?
• Are only explicitly mapped parameters allowed (MapTo (#103 (comment)) approach)?
• If not only explicitly mapped parameters are allowed, are the parameters handled the same as the source object (eg. only properties of the additional parameters are considered) or are they handled the same as a source object property?

I think handling these mappings the same as the generic mappings (they are never called by Mapperly generated code) with an explicitly mapped parameter (MapTo) approach would be the best for now. The added complexity is manageable while still addressing most of the use-cases. This would only allow additional parameters to be mapped to the target object directly.