Flow resolution context to nested mappings #2940
Conversation
| => _inner.Map(source, default(TDestination), _resolutionContext); | ||
|
|
||
| public TDestination Map<TSource, TDestination>(TSource source, Action<IMappingOperationOptions<TSource, TDestination>> opts) | ||
| { |
There was a problem hiding this comment.
There is some replication. It could be avoided by having the called Map methods always receive an optional context parameter.
There was a problem hiding this comment.
I don't want to add any more optional parameters to the public API, if i can avoid it. Those have backwards compat problems.
The biggest problems were the overloads taking Action<IMappingOperationOptions>. Those really shouldn't be allowed inside an IRuntimeMapper.
One option would be to separate the interface/objects for IMapper and IRuntimeMapper. Only put the things you should be able to do from the outermost mapping API on IMapper, and only the things you can do during a runtime mapping on IRuntimeMapper.
There was a problem hiding this comment.
That makes sense. It's weird right now.
src/AutoMapper/ResolutionContext.cs
Outdated
| @@ -105,7 +106,7 @@ internal int GetTypeDepth(TypePair types) | |||
| public ResolutionContext(IMappingOperationOptions options, IRuntimeMapper mapper) | |||
| { | |||
| Options = options; | |||
| Mapper = mapper; | |||
| Mapper = new RuntimeMapper(this, mapper); | |||
There was a problem hiding this comment.
That's an extra allocation for every Map call with context. It could be avoided by having the context privately implement IMapper and return this from the Mapper property. But I see why you might not want to do that :)
There was a problem hiding this comment.
Ha! I've already introduced a separate object - I'm not sure it's a terrible idea. I'll push a commit to see what it looks like.
There was a problem hiding this comment.
OK just pushed this. I think it works better - and in the future, we may want to remove the implicit interface implementation and the Mapper property, and clean up the interface to only the things that make sense for doing mappings inside a Map context.
Also we may have to rename this object. ResolutionContext is....a weird name.
|
A minor version bump? |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #2937