#3387 NullValuePropertyMappingStrategy for Maps and Iterables
#3411
Conversation
| assertThat( userDTO.getHomeDTO() ).isNotNull(); | ||
| assertThat( userDTO.getHomeDTO().getAddressDTO() ).isNull(); | ||
| assertThat( userDTO.getDetails() ).isNotNull(); |
There was a problem hiding this comment.
From what I understand:
-
assertThat( userDTO.getHomeDTO() ).isNotNull();:
should always succeed, even if weuserDTO.setHomeDTO( null ), because we are doing nested accessor (@Mapping(target = "homeDTO.addressDTO", source = "address")) -
assertThat( userDTO.getHomeDTO().getAddressDTO() ).isNull();:
should only succeed whencustomer.getAddressDTO() == null(SET_TO_NULL) -
assertThat( userDTO.getDetails() ).isNotNull();:
should always succeed (SET_TO_DEFAULT)
Is this correct, because last one is not succeeding and I will try to fix it.
There was a problem hiding this comment.
I think for this one I would have to understand how UpdateWrapper's Java class and ftl file are working.
Could you please give me some hints on how to debug the compilation of the ftl files to the generation of mapper impls?
I tried:
mvn test -Dmaven.surefire.debug -pl :mapstruct-processor -Dtest="*PropertyNameTest"To be able to connect to the debugger and see the generation step-by-step. But the debugger runs after the compilation step when the tests are going to start.
There was a problem hiding this comment.
@aboqasem Debugging in Maven tests is quite cumbersome, see: https://www.jetbrains.com/help/idea/work-with-tests-in-maven.html#debug_maven_goal
Why don't you just run the (e.g.) TargetPropertyNameTest inside the IDE with debug mode?
As soon as you run the test, you find the generated mappers (based on the ftl templates) inside the directory:
YOUR-WORKSPACE/mapstruct/processor/target/compilation-tests/org.mapstruct.ap.test.conditional.targetpropertyname.TargetPropertyNameTest/...
There was a problem hiding this comment.
Thanks for the response, @thunderhook!
For that I have no problem, I can see the issue in the generated files, it seems to ignore the newly added options, even though I followed what has been done in NullValueMappingStrategy for Maps and Iterables (it's always taking nullValuePropertyMappingStrategy).
And because I still do not get how things are working together I was wondering how I can debug the UpdateWrapper class.
I could not run the project in IntelliJ that's why I used the CLI 😀.
There was a problem hiding this comment.
Also had the problem to setup the project. I also have to comment out some classes (that I must not commit) to get it working inside IntelliJ.
What errors do you get in Intellij? Maybe I can help. Working inside the IDE makes it a lot easier.
There was a problem hiding this comment.
There was a problem hiding this comment.
I've got the same issue. I was told that this is due to some wrong language level settings inside the project modules. But I was not able to set it up correctly. If you find a solution, please let me know.
My workaround for this is the following patch, that I hold in a separate changeset that I do not commit. With this, the build (and all tests) are working. Tell me if that works for you, and have fun using the project inside IntelliJ 😉
Index: processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedTest.java
===================================================================
diff --git a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedTest.java b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedTest.java
deleted file mode 100644
--- a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedTest.java (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
+++ /dev/null (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
@@ -1,63 +0,0 @@
-/*
- * Copyright MapStruct Authors.
- *
- * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
- */
-package org.mapstruct.ap.test.annotatewith.deprecated.jdk11;
-
-import java.lang.reflect.Method;
-import org.mapstruct.ap.test.annotatewith.CustomMethodOnlyAnnotation;
-import org.mapstruct.ap.testutil.ProcessorTest;
-import org.mapstruct.ap.testutil.WithClasses;
-import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult;
-import org.mapstruct.ap.testutil.compilation.annotation.Diagnostic;
-import org.mapstruct.ap.testutil.compilation.annotation.ExpectedCompilationOutcome;
-import org.mapstruct.factory.Mappers;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-/**
- * @author orange add
- */
-public class DeprecatedTest {
- @ProcessorTest
- @WithClasses({ DeprecatedMapperWithMethod.class, CustomMethodOnlyAnnotation.class})
- public void deprecatedWithMethodCorrectCopyForJdk11() throws NoSuchMethodException {
- DeprecatedMapperWithMethod mapper = Mappers.getMapper( DeprecatedMapperWithMethod.class );
- Method method = mapper.getClass().getMethod( "map", DeprecatedMapperWithMethod.Source.class );
- Deprecated annotation = method.getAnnotation( Deprecated.class );
- assertThat( annotation ).isNotNull();
- assertThat( annotation.since() ).isEqualTo( "18" );
- assertThat( annotation.forRemoval() ).isEqualTo( false );
- }
-
- @ProcessorTest
- @WithClasses(DeprecatedMapperWithClass.class)
- public void deprecatedWithClassCorrectCopyForJdk11() {
- DeprecatedMapperWithClass mapper = Mappers.getMapper( DeprecatedMapperWithClass.class );
- Deprecated annotation = mapper.getClass().getAnnotation( Deprecated.class );
- assertThat( annotation ).isNotNull();
- assertThat( annotation.since() ).isEqualTo( "11" );
- }
-
- @ProcessorTest
- @WithClasses( { RepeatDeprecatedMapperWithParams.class})
- @ExpectedCompilationOutcome(
- value = CompilationResult.SUCCEEDED,
- diagnostics = {
- @Diagnostic(
- kind = javax.tools.Diagnostic.Kind.WARNING,
- type = RepeatDeprecatedMapperWithParams.class,
- message = "Annotation \"Deprecated\" is already present with the " +
- "same elements configuration."
- )
- }
- )
- public void bothExistPriorityAnnotateWithForJdk11() {
- RepeatDeprecatedMapperWithParams mapper = Mappers.getMapper( RepeatDeprecatedMapperWithParams.class );
- Deprecated deprecated = mapper.getClass().getAnnotation( Deprecated.class );
- assertThat( deprecated ).isNotNull();
- assertThat( deprecated.since() ).isEqualTo( "1.5" );
- assertThat( deprecated.forRemoval() ).isEqualTo( false );
- }
-}
Index: processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithMethod.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithMethod.java b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithMethod.java
--- a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithMethod.java (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
+++ b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithMethod.java (date 1696277894987)
@@ -13,7 +13,7 @@
public interface DeprecatedMapperWithMethod {
@AnnotateWith(CustomMethodOnlyAnnotation.class)
- @Deprecated(since = "18", forRemoval = false)
+ @Deprecated()
Target map(Source source);
class Source {
Index: processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithClass.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithClass.java b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithClass.java
--- a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithClass.java (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
+++ b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/DeprecatedMapperWithClass.java (date 1696277894988)
@@ -8,6 +8,6 @@
import org.mapstruct.Mapper;
@Mapper
-@Deprecated(since = "11")
+@Deprecated()
public class DeprecatedMapperWithClass {
}
Index: processor/src/main/java/org/mapstruct/ap/internal/model/AdditionalAnnotationsBuilder.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/AdditionalAnnotationsBuilder.java b/processor/src/main/java/org/mapstruct/ap/internal/model/AdditionalAnnotationsBuilder.java
--- a/processor/src/main/java/org/mapstruct/ap/internal/model/AdditionalAnnotationsBuilder.java (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
+++ b/processor/src/main/java/org/mapstruct/ap/internal/model/AdditionalAnnotationsBuilder.java (date 1696277894992)
@@ -10,7 +10,6 @@
import java.lang.annotation.Target;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
@@ -112,20 +111,20 @@
return annotations;
}
List<AnnotationElement> annotationElements = new ArrayList<>();
- if ( deprecatedGem.since() != null && deprecatedGem.since().hasValue() ) {
- annotationElements.add( new AnnotationElement(
- AnnotationElementType.STRING,
- "since",
- Collections.singletonList( deprecatedGem.since().getValue() )
- ) );
- }
- if ( deprecatedGem.forRemoval() != null && deprecatedGem.forRemoval().hasValue() ) {
- annotationElements.add( new AnnotationElement(
- AnnotationElementType.BOOLEAN,
- "forRemoval",
- Collections.singletonList( deprecatedGem.forRemoval().getValue() )
- ) );
- }
+// if ( deprecatedGem.since() != null && deprecatedGem.since().hasValue() ) {
+// annotationElements.add( new AnnotationElement(
+// AnnotationElementType.STRING,
+// "since",
+// Collections.singletonList( deprecatedGem.since().getValue() )
+// ) );
+// }
+// if ( deprecatedGem.forRemoval() != null && deprecatedGem.forRemoval().hasValue() ) {
+// annotationElements.add( new AnnotationElement(
+// AnnotationElementType.BOOLEAN,
+// "forRemoval",
+// Collections.singletonList( deprecatedGem.forRemoval().getValue() )
+// ) );
+// }
annotations.add( new Annotation(deprecatedType, annotationElements ) );
return annotations;
}
Index: processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/RepeatDeprecatedMapperWithParams.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/RepeatDeprecatedMapperWithParams.java b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/RepeatDeprecatedMapperWithParams.java
--- a/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/RepeatDeprecatedMapperWithParams.java (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
+++ b/processor/src/test/java/org/mapstruct/ap/test/annotatewith/deprecated/jdk11/RepeatDeprecatedMapperWithParams.java (date 1696277894972)
@@ -9,7 +9,7 @@
import org.mapstruct.Mapper;
@Mapper
-@Deprecated( since = "1.8" )
+@Deprecated( )
@AnnotateWith(
value = Deprecated.class,
elements = {
Index: processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java b/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java
--- a/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java (revision ea997f83cefd7d3900f8a6a218af1c20f7a1ef8a)
+++ b/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java (date 1696277894991)
@@ -13,13 +13,12 @@
import java.util.ListIterator;
import java.util.Map;
import javax.annotation.processing.ProcessingEnvironment;
-import javax.lang.model.SourceVersion;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
-import javax.lang.model.element.ModuleElement;
+//import javax.lang.model.element.ModuleElement;
import javax.lang.model.element.Name;
import javax.lang.model.element.PackageElement;
import javax.lang.model.element.TypeElement;
@@ -46,13 +45,13 @@
@IgnoreJRERequirement
AbstractElementUtilsDecorator(ProcessingEnvironment processingEnv, TypeElement mapperElement) {
this.delegate = processingEnv.getElementUtils();
- if ( SourceVersion.RELEASE_8.compareTo( processingEnv.getSourceVersion() ) >= 0 ) {
+// if ( SourceVersion.RELEASE_8.compareTo( processingEnv.getSourceVersion() ) >= 0 ) {
// We are running with Java 8 or lower
this.moduleElement = null;
- }
- else {
- this.moduleElement = this.delegate.getModuleOf( mapperElement );
- }
+// }
+// else {
+// this.moduleElement = this.delegate.getModuleOf( mapperElement );
+// }
}
@Override
@@ -63,7 +62,8 @@
}
// If the module element is not null then we must be running on Java 8+
- return this.delegate.getPackageElement( (ModuleElement) moduleElement, name );
+// return this.delegate.getPackageElement( (ModuleElement) moduleElement, name );
+ return null;
}
@Override
@@ -74,7 +74,8 @@
}
// If the module element is not null then we must be running on Java 8+
- return this.delegate.getTypeElement( (ModuleElement) moduleElement, name );
+// return this.delegate.getTypeElement( (ModuleElement) moduleElement, name );
+ return null;
}
@Override
There was a problem hiding this comment.
Thanks a lot @thunderhook! I tried this and it still did not work, will try to post the error when I get back to my machine.
There was a problem hiding this comment.
Please do not use this patch, I found another solution, see #3461 (comment).
aboqasem
force-pushed
the
feat/#3387-maps-and-iterable-null-value-property-mapping-strategy
branch
from
e8a8aaf to
932ca9b
Compare
|
@aboqasem if you are using Draft then I think that the tests are not running. I haven't reviewed the entire PR, but I am curious to know why do we need |
In discussion #3380 (started by me) you wrote
The motivation for this was the wish to introduce a new
This is precisely the problem that is to be solved. Moreover, as I wrote in #3381 (comment) (at the bottom), the distinction between normal and update mappings should perhaps be reconsidered, since a normal mapping should be nothing else than an update mapping targeting a newly created POJO/DTO. |
|
Yes I did write that, but I do not think that it was for the For this to work on @Mapper
public interface MyMapper {
void map(List<Source> source, @MappingTarget List<Target> target);
}This is something that we currently do not support. |
|
@filiphr Sorry, I misread the thread. |
|
Hello, @filiphr. Apologies for the inactivity, as I haven't sat down to thoroughly understand the code, I thought it would be a simple change that would lead to partially understanding the codebase. I was just looking for where I think I will close the PR to give it to someone else, I will try to understand the codebase and hopefully start contributing. |
|
@aboqasem no need to apologise and my intentions were not to discourage you from contributing, quite the contrary actually. I think that the PR was in the right direction. Only the additions in |
I have no doubt about that!
The issue is just that I'm not sure what I'm doing haha! I thought by just following what was done in the previous PR, I would understand the codebase and also implement the feature. But turns out there are concepts I'm not familiar with how to approach them that would take me some time to grasp (e.g. code generation). |
4 participants
Closes #3387.
Hello, this is my first PR to
mapstruct.Opening a draft PR until I: