-
Notifications
You must be signed in to change notification settings - Fork 380
Add enhanced Mapster dependency injection system with fluent configuration API #819
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
Conversation
- Added new DI extension methods in `ServiceCollectionExtensions.cs`: `AddMapster` with options, `AddMapsterWithConfig`, `AddMapsterFrozen`, `AddMapsterModule`, and `ScanMapster` for assembly scanning. - Introduced `MapsterOptions` for flexible configuration, including assembly scanning, precompilation, and module registration. - Added `IMapContextFactory` and `DefaultMapContextFactory` for managing `MapContext` scopes. - Created `MapsterBuilder` and `MapsterBuildActions` to handle build phases, including precompilation and freezing configurations. - Enhanced `MapsterBuilder` to support `IMapFrom<T>` pattern and improved error handling during precompilation. - Added unit tests in `EnhancedDITests.cs` to validate new DI features. - Added `StartupEnhanced.cs` to demonstrate advanced Mapster DI usage. - Introduced `User` and `UserDto` models for testing and examples. - Preserved backward compatibility with the original `AddMapster` method.
|
This is a somewhat peculiar PR. A couple of questions;
@DocSvartz @stagep What are your thoughts on this? |
|
@andrerav A few thoughts. |
|
Thanks for the feedback! |
|
@andrerav I don't mind adding a fluent configuration api. @mokarchi Are all these features aimed at simplifying migration from AutoMapper to Mapster? I didn't quite understand the difference between Frozen and Precompile config. Both of these extensions perform precompilation. According to the description, freezing should prevent the configuration from being changed again. |
To answer your question about the goal: Yes, a key aim of this PR is to simplify migration from AutoMapper to Mapster by offering a familiar, fluent API (like inline config and builder patterns). This makes it easier for AutoMapper users to adopt Mapster with minimal code changes. However, it also benefits existing Mapster users by addressing pain points like manual config setup outside DI and adding performance optimizations like precompile and freeze without requiring source generators |
Regarding your question about Frozen vs. Precompile:
|
|
OK, |
|
Given your feedback and the concerns raised by @stagep about the scope of this PR, IIf you and the team think this PR doesn’t fit Mapster’s core at this time, I’m fully supportive of closing it and will use your feedback to refine my approach for future contributions if you feel it’s not the right fit for Mapster’s core at this time. My goal was to address user needs while maintaining Mapster’s simplicity and performance focus, but I respect that this might require more discussion. |
|
@mokarchi Don't make any changes. We are just discussing ideas. I specifically started my first comment with "a few thoughts". I am not making any judgment of good or bad. I was just throwing a few things out there to discuss and that is what we are doing. @DocSvartz has been the major code contributor to Mapster recently. I generally answer "support" related issues that deal with configuration or issues that do not involve code changes. Personally, I didn't come from AutoMapper, but I know about their licensing changes, so I can understand where you are coming from. I also know that fluent configuration is preferred by some rather than imperative configuration. Let's keep the discussion going @DocSvartz @andrerav |
|
I would argue that a change like this should start its life as an extension to Mapster, not as core functionality. One possible compromise is that we create a new repository in the Mapster organization (for example .../Mapster.Fluent) and give @mokarchi write access to that repository. When the extension is proven to be reliable and useful, we can consider including it in the main repository. What do you think @stagep and @DocSvartz? |
|
@andrerav @DocSvartz I like that idea because it guarantees that the Mapster core is not touched, and that the addition of fluent configuration is 100% an extension. |
|
@andrerav I like that idea. |
|
I completely agree that starting this as an extension (like in a new repo under the Mapster organization, e.g., Mapster.Fluent) is a great compromise to keep the core untouched while testing its reliability and usefulness. If the team decides to go that route, I'm fully on board and would be thrilled to have write access to maintain and iterate on it. I'm excited to support it and contribute in a way that aligns with Mapster's goals. Let me know how I can help next! @DocSvartz @andrerav |
|
@DocSvartz @stagep @mokarchi I think the config freeze option should either be anchored in a security concern (which I can't really surmise from the current proposal), or a benchmark that shows a clear and tangible performance advantage. |
|
@andrerav My expectation of freezing mostly coincides with the description @mokarchi gave above. I see two solutions:
@andrerav @stagep @mokarchi Perhaps you see another solution? Upd: For now, can inherit from TypeAdapterConfig. |
|
Regarding @DocSvartz’s question about whether Freeze is needed outside DI cases: I believe Freeze can be valuable in both DI (ServiceMapper) and non-DI (Mapper) scenarios. In non-DI cases, Freeze ensures that the TypeAdapterConfig remains immutable after setup, preventing accidental modifications during runtime that could lead to configuration errors (as noted in the wiki). This is particularly useful in long-running apps where config stability is critical. For DI scenarios, it also enables a fast-path O(1) lookup by caching compiled delegates, which aligns with Mapster’s performance focus. @andrerav, I completely agree that Freeze needs a clear justification, either through a security concern or a tangible performance benefit. From a security perspective, making TypeAdapterConfig immutable prevents unintended runtime changes that could introduce bugs or inconsistencies, especially in production environments with multiple threads or services accessing the config. For performance, Freeze enables O(1) lookup by pre-caching compiled delegates and skipping dynamic resolution, which can be a bottleneck in high-throughput scenarios. @DocSvartz, I really appreciate your proposed solutions for implementing Freeze. I agree that the lack of immutability in TypeAdapterConfig can lead to configuration failures, as you mentioned. Here’s my take on your suggestions: Adding a freezing/blocking feature directly to TypeAdapterConfig: This seems like the most straightforward approach. We could add an IsFrozen flag and guard mutating methods (e.g., ForType(), NewConfig()) to throw an InvalidOperationException if called post-freeze. This keeps the core simple and aligns with Mapster’s philosophy. |
|
@mokarchi Create a prototype based on inheritance. |
Before I proceed, could you clarify where you’d like me to create this prototype? |
|
@mokarchi I've created a new repository here: https://github.com/MapsterMapper/Mapster.Fluent Please fork it, create your changes, and submit a PR. And we can continue the technical discussion on that PR. |
|
@mokarchi The inheritance based approach won't work now either. |
|
@DocSvartz How so? I think it should work fine to add a package reference to Mapster and inherit from there? |
|
@DocSvartz, You're right the inheritance-based approach doesn't work as expected because the methods in TypeAdapterConfig aren't virtual, making it impossible to override them for immutability. I've been working on an alternative solution (using a decorator pattern with a custom interface to handle the guards and freeze logic), and I'll submit a PR to Mapster.Fluent soon to address this. |
|
@andrerav will need to make the methods virtual. |
|
@DocSvartz Oh, yes of course 🤦 But I can see in his fork that he has apparently found a way around it. |
|
@mokarchi Your approach will only work with inline config. When uploading assemblies, you will still pass the standard config, which not blocking. |
|
@DocSvartz, I completely agree that the current approach only works for inline config and doesn't handle scanned assemblies properly, as it passes the standard TypeAdapterConfig without blocking mutations. You're right. this limits the immutability to inline setups. I'm working on extending the wrapper to cover assemblies as well (e.g., by applying the decorator after scanning), and I'll submit a PR to Mapster.Fluent soon to address this. If you have any specific suggestions on how to handle this better, I'd love to hear them |
|
@mokarchi I don't have any good thoughts for the current situation yet. The only thing is, if the configuration is already frozen at the time of scanning, then there is no need to scan. But you had a different order, first scan, then checking for freezing. |
|
@andrerav @DocSvartz @mokarchi Just my two cents, but shouldn't the focus first be on the extension methods that provide fluent configuration before we get into the nitty-gritty of specifics of Freeze etc. Let's deliver what can bring immediate value and then we can discuss more specific edge cases. |
|
@stagep, I agree that focusing on delivering immediate value with fluent extension methods makes sense, and we can tackle edge cases like Could you please clarify what specific features you'd like to see prioritized in the first version of Mapster.Fluent? |
|
@stagep I vote roughly for the following distribution:
|
100% agree |
|
Sounds good to me as well :) |
|
Please review and share feedback. PR |
|
Updated PR became MapsterMapper/Mapster.Fluent#1 |
Overview
This update significantly enhances the Mapster dependency injection system with a comprehensive configuration API inspired by AutoMapper's
AddAutoMappermethod, while maintaining full backward compatibility with existing code. This package provides enhanced dependency injection and configuration support for Mapster, making it easier to integrate with ASP.NET Core applications.Key Improvements
1. Fluent Configuration API
Instead of manually creating and configuring
TypeAdapterConfigoutside of DI, you can now configure Mapster inline:2. Assembly Scanning
Automatically discover and register mapping configurations:
Supports both existing patterns:
IRegister- Existing pattern for mapping configurationsIMapFrom<T>- New pattern allowing DTOs to define their own mappings3. ServiceMapper by Default
The new API uses
ServiceMapperby default, providing full dependency injection support in mappings:4. Precompilation Support
Precompile frequently used mappings at startup for better runtime performance:
5. Frozen Configuration
Lock configuration for maximum performance in production:
6. Module System
Create reusable, testable mapping configurations:
7. Build Hooks
Execute logic before and after compilation:
8. MapContext Factory
Customize how MapContext scopes are created:
Quick Start
Basic Usage (Backward Compatible)
With Inline Configuration
With Options
Frozen Configuration (High Performance)
With Build Actions
Using Modules
Assembly Scanning
Pattern Support
IRegister Pattern (Existing)
IMapFrom Pattern (New)
Advanced Scenarios
Custom MapContext Factory
Combining Multiple Configurations
API Reference
MapsterOptions
AssembliesToScan: Assemblies to scan for mapping configurationsTypePairsToPrecompile: Type pairs to precompileUseServiceMapper: Use ServiceMapper (true) or basic Mapper (false)FreezeConfiguration: Freeze config after buildConfigureAction: Action to configure TypeAdapterConfigModules: Modules to registerMapsterBuildActions
OnBeforeCompile: Action executed before compilationOnAfterCompile: Action executed after compilationScanAssemblies: Enable assembly scanningPrecompileTypePairs: Enable type pair precompilationFreeze: Freeze configurationExtension Methods
AddMapster(): Basic registration (backward compatible)AddMapster(Action<MapsterOptions>): Register with optionsAddMapster(Action<TypeAdapterConfig>, Action<MapsterBuildActions>): Register with config and actionsAddMapsterWithConfig(TypeAdapterConfig, Action<MapsterBuildActions>): Use existing configAddMapsterFrozen(Action<TypeAdapterConfig>): Register with frozen configAddMapsterModule<TModule>(): Register a moduleScanMapster(params Assembly[]): Scan assembliesMigration Guide
Minimal Migration (No Changes Required)
Your existing code continues to work without any changes:
Recommended Migration
To take advantage of new features, update your code to:
Full Feature Migration
For maximum benefits:
Mapster.DependencyInjection - Usage Examples
Example 1: Simple ASP.NET Core Setup
Example 2: Assembly Scanning with IRegister
Example 3: Using IMapFrom Pattern
Example 4: Dependency Injection in Mappings
Example 5: High-Performance Frozen Configuration
Example 6: Precompiling Hot Paths
Example 7: Modular Configuration
Example 8: Build Hooks for Logging
Example 9: Combining Multiple Configurations
Example 10: Using Existing Configuration
Example 11: Conditional Configuration by Environment
Example 12: Testing Configuration
Backward Compatibility
AddMapster()method preservedoptions.UseServiceMapper = false