AutoMapper / AutoMapper.Extensions.Microsoft.DependencyInjection

MIT License
258 stars 79 forks source link

Allow AddAutoMapper to be called multiple times, is it possible ? #118

Closed DavidNorena closed 4 years ago

DavidNorena commented 4 years ago

Hi guys we are working with clean architecture in some APIS, and also we are working multiple in-house libraries, is there a way that we can call Multiple times AddAutoMapper from different libraries, so each library registrers is own Profiles ....

Remember also that exists this method that you could use here, so in that case you don't need to return here.

jbogard commented 4 years ago

That’s not the problem, the mapping configuration is Singleton and all profiles need to be loaded in that one call.

Why do your libraries need to call AddAutoMapper? Why can’t they just declare Profiles?

On Tue, Oct 15, 2019 at 2:29 PM David Noreña Perez notifications@github.com wrote:

Hi guys we are working with clean architecture in some APIS, and also we are working multiple in-house libraries, is there a way that we can call Multiple times AddAutoMapper from different libraries, so each library registrers is own Profiles ....

Remember also that exists this method https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.extensions.servicecollectiondescriptorextensions.tryadd?view=dotnet-plat-ext-3.0 that you could use here https://github.com/AutoMapper/AutoMapper.Extensions.Microsoft.DependencyInjection/blob/master/src/AutoMapper.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs#L98, so in that case you don't need to return here https://github.com/AutoMapper/AutoMapper.Extensions.Microsoft.DependencyInjection/blob/master/src/AutoMapper.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs#L63 .

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/AutoMapper/AutoMapper.Extensions.Microsoft.DependencyInjection/issues/118?email_source=notifications&email_token=AAAZQMUDRPBSZDV4BLLOMUTQOYK3FA5CNFSM4JBBBKZKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4HR64UEQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZQMRQWFYDLE7OX2WLB23QOYK3FANCNFSM4JBBBKZA .

DavidNorena commented 4 years ago

Well we were trying to achieve just use the library with AddLibrary() -> and that method will add automapper, so we dont have to worry about AddAutoMapper(typeof(ProfileLybrary1), typeof(ProfileLybrary2), typeof(ProfileLybrary2)) outside the libraries, I hope you get my idea ....

lbargaoanu commented 4 years ago

The other option is for each library to have its own MapperConfiguration.

jbogard commented 4 years ago

@DavidNorena so the fundamental problem isn't in this library, but instead it's that MapperConfiguration is singleton and requires all maps be registered up front.

The only way I can see around this is having some lazily evaluated singleton, and the first time it's evaluated, it gathers all of the types/assemblies you've configured.

Not a bad idea, but it'll be a bit complicated, we'll have to register some type/assembly collector.

DavidNorena commented 4 years ago

Lol, yeah I know is not that simple, I can help if you want, the real question is now, should I keep this issue opened ? and work on this ?

jbogard commented 4 years ago

I think some research is needed for how other libraries handle this (if at all). I'd like us to be consistent with other folks at least.

lbargaoanu commented 4 years ago

Perhaps this should be implemented elsewhere. Most people don't need it. And most likely it will break some existing usages.

DavidNorena commented 4 years ago

Well the idea is the same usage AddAutoMapper() the change could be internally, so wouldn't break anything at all

lbargaoanu commented 4 years ago

I understand the idea :) Just that we break things all the time in ways we didn't even imagine. I would be very surprised if this kind of change would be any different.

lbargaoanu commented 4 years ago

Perhaps a new extension method would work.

ndobryanskyy commented 4 years ago

@lbargaoanu @jbogard I think, that this scenario could be implemented in the standard Options fashioned way. To do that auto-mapper could expose some class for options configurations something that derives IMapperConfigurationExpression and will include the marker types and assemblies to scan.

In this way any other assembly could call

services.Configure(expression => expression.AddAssembly(GetType().Assembly));
// Or
services.Configure(expression => expression.AddMarkerType(typeof(Marker)));

Also this change will not break any existing code, but rather will add the new way to contribute to auto mapper configuration from any assembly.

And in the AddAutomapper code these options could be resolved and used to register all of the types required to auto mapper.

https://github.com/AutoMapper/AutoMapper.Extensions.Microsoft.DependencyInjection/blob/ee8fcde200a31c28ef0299e09284efc5e9b8ab4b/src/AutoMapper.Extensions.Microsoft.DependencyInjection/ServiceCollectionExtensions.cs#L97

Does that make sense?

lock[bot] commented 4 years ago

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.