#3309 - Add BeanMapping#unmappedSourcePolicy
#3326
Conversation
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
Thanks for your work on this @venkatesh2090.
Have a look at my comments. In addition to that, can you please add tests with different policies and also a combination of policy on @Mapper and @BeanMapping to show that it is being picked up properly
processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java
Show resolved
Hide resolved
processor/src/main/java/org/mapstruct/ap/internal/model/BeanMappingMethod.java
Show resolved
Hide resolved
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
Thanks for the changes. I've added one last comment and we'll merge once that is done and the build is green
| /** | ||
| * How unmapped properties of the source type of a mapping should be reported. | ||
| * If no policy is configured, the policy given via {@link MapperConfig#unmappedSourcePolicy()} or | ||
| * {@link Mapper#unmappedSourcePolicy()} will be applied, using {@link ReportingPolicy#WARN} by default. |
There was a problem hiding this comment.
| * {@link Mapper#unmappedSourcePolicy()} will be applied, using {@link ReportingPolicy#WARN} by default. | |
| * {@link Mapper#unmappedSourcePolicy()} will be applied, using {@link ReportingPolicy#IGNORE} by default. |
|
I was refactoring the tests and writing more which broke more stuff. I'll fix it and push it once it's done. FYI here's the error I get during the tests. ERROR BeanMappingSourcePolicyMapper.java:20 'nullValueMappingStrategy', 'nullValuePropertyMappingStrategy', 'resultType' and 'qualifiedBy' are undefined in @BeanMapping, define at least one of them.It might take a bit more time. My bad. |
|
I did manage to fix it, but I have one concern. This error message in Message looks outdated. It is being used in BeanMappingOptions.isConsistent where it checks for more values than the error message points out. The message is inconsistent I think. |
filiphr
left a comment
filiphr
left a comment
There was a problem hiding this comment.
Thanks @venkatesh2090, one last comment for the integration tests to pass and we are done. You are right, the error message in the @BeanMapping is inconsistent. We can change this in a different PR. The message doesn't need to enumarate all options, we can just say @BeanMapping must have at least one option defined
| import org.mapstruct.ReportingPolicy; | ||
|
|
||
| @Mapper(unmappedSourcePolicy = ReportingPolicy.WARN) | ||
| public interface BeanMappingSourcePolicyErrorMapper { |
There was a problem hiding this comment.
This should contain ErroneousMapper, e.g. BeanMappingSourcePolicyErroneousMapper. Doing it like this will make sure that the integration tests will ignore this mapper
|
Thanks @venkatesh2090, this has been integrated into main |
2 participants
Add support for specifying
unmappedSourcePolicysimilar tounmappedTargetPolicyonBeanMapping.Fixes #3309