Separate RequiredMappingStrategy configuration for enums

[jorisBarkema]

Is your feature request related to a problem? Please describe. When there are multiple properties on the Source you want to ignore because they are not present (and relevant on the target) setting RequiredMappingStrategy = RequiredMappingStrategy.Target is super convenient. However, setting this property also removes the warnings when the source has Enum values that are not present on the Target, which can cause runtime exceptions.

Describe the solution you'd like I would like the RequiredMappingStrategy.Target to still give warnings when the source has enum values which are not present in the target.

Describe alternatives you've considered The alternative is not using RequiredMappingStrategy.Target and ignoring the members not present on the source manually using [MapperIgnoreSource] but that is much less convenient in many cases.

Additional context N/A

[jacob-buckaroo]

I submitted a PR that removes the constraints on "RequiredMappingStrategy.Target" and "RequiredMappingStrategy.Source" related to missing target/source enum members.

My reasoning is that RequiredMappingStrategy should only affect missing properties or fields, rather than limiting the possible values within an enum.

[latonz]

I don't think it's a good idea to disable this for all enum mappings. I think for a lot of users this can be a helpful feature. This would also be a breaking change and cannot be implemented in a minor release. You can already overwrite the required mapping strategy for each mapping, including enum mappings. What may be helpful is a new default for enum mappings which would fall back to the general setting if it is not set.

[jacob-buckaroo]

I'm not sure if we're on the same page. My PR does not break any tests.

[latonz]

Unfortunately, the test coverage is insufficient in this case. I have added a test #1593 that fails with the proposed changes.

[jorisBarkema]

Right now, if I understand how the mappings work correctly, setting RequiredMappingStrategy.Target can lead to unexpected run-time exceptions for any Enum values that are present in the source but not on the Target. This is fundamentally different from how this attribute property works for Properties, because say you are mapping A to B and A has a property that B does not have, that's absolutely fine. I'm not fully clear on what you mean with this

What may be helpful is a new default for enum mappings which would fall back to the general setting if it is not set.

But in my opinion it would make more sense to fully split the properties for required mapping strategies of properties and enums because they work differently. So that would mean for this feature to add something like a RequiredEnumMappingStrategy or something, and use that for Enums instead of the existing RequiredMappingStrategy. If you want to prevent breaking changes we could also let this override RequiredMappingStrategy instead, so that everything that does not implement RequiredEnumMappingStrategy keeps working like it does now.

You can already overwrite the required mapping strategy for each mapping, including enum mappings.

I did not know that, but it does not fully solve the issue in my case at least. One of the main reasons I want to use Mapperly and not something like AutoMapper is that this uses source generation, and because of that can give much more validation at build-time instead of run-time. Doing it this way would require us to always set RequiredMappingStrategy.Both for all Enums, and if you forget it once, you can get these run-time exception we want to avoid.

[latonz]

Sorry if I wasn't clear on my first comment. I totally understand where you are coming from. What you are describing is more or less what i meant. Idea on how the current situation could be improved without a breaking change:

• Add MapperAttribute.RequiredEnumMappingStrategy / MapperDefaultsAttribute.RequiredEnumMappingStrategy
• Resolve RequiredMappingStrategy for each enum mapping in the following order, fall back to the next one if the previous one is not set: 1.MapperRequiredMappingAttribute on the mapping method
  2. MapperAttribute.RequiredEnumMappingStrategy
  3. MapperDefaultsAttribute.RequiredEnumMappingStrategy
  4. MapperAttribute.RequiredMappingStrategy
  5. MapperDefaultsAttribute.RequiredMappingStrategy

With the next breaking change it would then be possible to decouple the enum strategy and remove 4 and 5 in the list. Would be happy to review a PR implementing this 😊