Allow running AutoMapper configuration in plugins

[andrewlock]

Summary

We need a way to configure AutoMapper in plugins. I propose to push AutoMapperConfiguration into Nop.Core and add an AddConfiguration() method.

Explanation

I have just been working through upgrading our nopcommerce plugins from 3.70 to 3.80 and have run into one significant issue.

Previously, we were using AutoMapper extensively to map our Entities to ViewModels etc. We ran the configuration code in an IStartupTask that would do all the appropriate CreateMap calls which worked smoothly.

In the latest upgrade to AutoMapper 5.0, as I'm sure you realise, the static Mapper interface has been replaced with an instance method, and has a requirement for all configuration to be performed as part of a single initialisation method.

Currently you have a IStartupTask that performs all the configuration in the Nop.Admin project by calling AutoMapperConfiguration.Init(). Somehow we need a decoupled way of providing additional configuration to the AutoMapperConfiguration.Init method.

There's a number of ways this could work, but one possibility would be to push the static AutoMapperConfiguration class down the project stack and into Nop.Core. It could have a public collection of configuration methods that IStartupTasks can add to, or better still an AddConfiguration(Action<IMapperConfigurationAction> configureAction) method that pushes into a private list.

You could then call AutoMapperConfiguration.Init() in NopEngine.Initialize() once all IStartupTasks have run, which will create a new MapperConfiguration using the provided configActions, and call CreateMapper().

It still feels a little messy, but I think it's possibly the cleanest solution?

I'm happy to put together a pull request if you agree.

[andrewlock]

Hi Andrei,

Thanks for looking at this. I have put together a pull request showing my proposal for your interest.

The only downside I can see is the need to reference AutoMapper in Nop.Core, but as long as you are not concerned about that (and I don't see any reason to be) then it should solve the plugin configuration problem.

Looking forward to your input!

[AndreiMaz]

Hi Andrew,

I've just played with it to reproduce the issue. I've added one more AutoMapperStartupTaskTest and AutoMapperConfigurationTest to Nop.Web (not to Nop.Admin). And it worked fine. Both configurations were successfully registered (AutoMapperConfiguration from Nop.Admin and the new one - AutoMapperConfigurationTest)

[andrewlock]

Thanks Andrei. Just to clarify, are you saying it worked fine on develop (as in, you couldn't reproduce the problem) or that it worked fine with the pull request?

[AndreiMaz]

Contribution #1756

[AndreiMaz]

@andrewlock I test it on my local computer (before I've seen your pull request)

[andrewlock]

Apologies Andrei, I'm not sure if you've understand the problem correctly, but do let me know if I'm just misunderstanding you!

If you call AutoMapperConfiguration.Init() in AutoMapperStartupTaskTest, then that will work fine in your AutoMapperConfigurationTest when run in isolation. However at runtime, the two AutoMapperStartup tasks will conflict, both calling Init() - the last IStartupTask to run will replace the MapperConfiguration and IMapper with its own registrations, causing runtime exceptions when you try to map anything from the other project.

Instead, we need to ensure all configurations are applied before calling _mapperConfiguration.CreateMapper() in AutoMapperConfiguration.Init(), and also that the Mapper and MapperConfiguration properties cannot be overwritten;

The other problem as it stands is that every project and plugin that uses AutoMapper would need to reference the Nop.Admin project, in order to reference the AutoMapperConfiguration class.

I think my pull request deals with both of these issues. 😄

[AndreiMaz]

Andrew, I see now. Thanks a lot! I'll have a look

[AndreiMaz]

Andrew, done. Again thanks a lot for suggestion!

P.S. I've refactored the implementation a bit. And made it more consistent with our other approaches (e.g. dependency registar)

[andrewlock]

Excellent, you've definitely nailed a better implementation, thanks! 😄