Mapping immutable types #216 #218

piotrdytkowski · 2014-12-11T10:09:58Z

Allows to use no-args constructor to create immutable objects

garethahealy · 2017-04-11T09:17:52Z

@piotrdytkowski ; can you rebase with latest to force travis to retest

garethahealy · 2017-05-25T17:08:04Z

@piotrdytkowski ; any update?

piotrdytkowski · 2017-05-26T07:42:37Z

Hey, sure, will have a look later. I don't like the name 'omit-constructor' anymore though...

garethahealy · 2017-06-22T08:27:46Z

@piotrdytkowski ; have you had anytime to progress?

piotrdytkowski · 2017-06-27T18:55:42Z

Hey, I rebased the project, but now have a test failing (VariablesTest) in maven but passing in my IDE, must be something with deps or environment. I hope to sort it out soon.

piotrdytkowski · 2017-06-27T19:03:16Z

Now I see that master branch CI is failing for the same reason. I will update when you fix/revert master.

garethahealy · 2017-07-05T10:29:23Z

@piotrdytkowski ; master branch has now been fixed. Can you rebase.

piotrdytkowski · 2017-07-05T11:31:45Z

I rebased but got some osgi tests failing, they seem unrelated but I can be wrong. Can you help me with this?

Tests in error: 
  OsgiContainerTest.shouldInstantiate:59 Â» ClassNotFound org.dozer.DozerBeanMapp...
  OsgiContainerTest.shouldLoadMappingFile:72 Â» ClassNotFound org.dozer.DozerBean...
  OsgiContainerTest.shouldMap:65 Â» ClassNotFound org.dozer.DozerBeanMapperBuilde...

piotrdytkowski · 2017-07-11T07:40:23Z

Hey @garethahealy , did you have any time to look at this?

garethahealy · 2017-07-11T08:29:52Z

@piotrdytkowski ; no sorry, am currently on holiday, so might be a few weeks.

garethahealy · 2017-08-06T13:20:46Z

core/pom.xml

+            <artifactId>junit</artifactId>
+            <scope>test</scope>
+            <version>4.12</version>
+        </dependency>


junit isnt required. It comes from the parent pom, as per:

https://github.com/DozerMapper/dozer/blob/master/plugins-parent/pom.xml#L83-L87

garethahealy · 2017-08-06T13:22:31Z

core/pom.xml

+            <groupId>org.apache.maven.surefire</groupId>
+            <artifactId>surefire-junit4</artifactId>
+            <version>2.20</version>
+        </dependency>


What uses this dep? Cant see why its needed.

garethahealy · 2017-08-06T13:24:30Z

schema/pom.xml

+            <groupId>org.apache.maven.surefire</groupId>
+            <artifactId>surefire-junit4</artifactId>
+            <version>2.20</version>
+        </dependency>


Same as other comment. Why are these 2 needed?

garethahealy · 2017-08-06T13:26:16Z

@piotrdytkowski ; the OSGi test are failing as you've added a new dependancy: org.objenesis.

It needs to be added as a required dependancy in the OSGi tests:

https://github.com/DozerMapper/dozer/blob/master/osgi-test/src/test/java/com/github/dozermapper/osgitests/OsgiContainerTest.java#L56-L61

piotrdytkowski · 2017-11-15T22:29:26Z

dozer-osgi-tests has a dependency on dozer-core so it should pull objenesis just fine. I rebased again and removed test libs added before. Hope to merge it soon 🍾

garethahealy · 2017-11-16T16:46:43Z

Like i said, you need to add the new dep to the OSGi container. Since my last comment, this has slightly changed, but if you look at:

https://github.com/DozerMapper/dozer/blob/master/tests/dozer-osgi-tests/src/test/java/com/github/dozermapper/osgitests/karaf/BundleOptions.java#L26

You'll see all the core deps needed. Those "link" URLS come from build time in:

target/generated-test-resources/alta/

piotrdytkowski · 2017-11-16T18:17:03Z

Great, that's what I needed. Thanks :)

piotrdytkowski · 2017-11-16T18:50:31Z

Build is green.

dmitrySDK · 2017-11-17T11:52:54Z

Hi! I think it is great functionality, but I cannot find a way to configure skipConstructor without XML, that is only with API. Is that possible with current code or (if not) maybe it should be a different PR?

BR
Dmitry Sherstnyakov

piotrdytkowski · 2017-11-17T12:26:21Z

I didn't have a look at the api, back in 2014 when I implemented it maybe api configuration didn't even exist :D
I would suggest to get it merged and then build on top of it. Maybe I'll do it when I have some free time.

garethahealy · 2017-11-18T17:56:03Z

core/pom.xml

+        <dependency>
+            <groupId>org.objenesis</groupId>
+            <artifactId>objenesis</artifactId>
+            <version>2.6</version>


Can you add this to the dependancyManagement section of the bom

https://github.com/DozerMapper/dozer/blob/master/bom-dependencies/pom.xml

garethahealy · 2017-11-18T18:01:07Z

CC @orange-buffalo

garethahealy · 2017-11-18T18:01:40Z

Looks fine from my point of view.

orange-buffalo

@piotrdytkowski It would be great to extend the documentation (see docs/asciidoc/documentation) with new feature description and example.

@garethahealy with objenesis dependency added, Dozer now pulls quite some list of dependencies into user's projects. Probably we should think about improvement here. Maybe creating one single uber jar with shaded dependencies could be an option..

orange-buffalo · 2017-11-19T09:49:54Z

core/src/main/java/org/dozer/classmap/DozerClass.java

+    return skipConstructor != null && skipConstructor;
+  }
+
+  public void setSkipConstructor(Boolean omitConstructor) {


It would be nice to rename the method parameter to match the property name.

orange-buffalo · 2017-11-19T09:52:02Z

core/src/main/java/org/dozer/loader/DozerBuilder.java

@@ -511,6 +511,10 @@ public void mapEmptyString(Boolean value) {
    public void isAccessible(Boolean value) {
      definition.setAccessible(value);
    }
+
+    public void isSkipConstructor(Boolean skipConstructor) {


I would recommend to rename this method to skipConstructor as it reflects the underlying configuration option more accurately.

orange-buffalo · 2017-11-19T09:59:13Z

core/src/main/java/org/dozer/factory/ConstructionStrategies.java

+        }
+
+        private <T> T newInstance(Class<T> clazz) {
+            ObjectInstantiator<T> instantiator = objectFactory.getInstantiatorOf(clazz);


Not sure if we are not loosing the semantics here. According to the javadocs, org.objenesis.Objenesis#getInstantiatorOf:

Will pick the best instantiator for the provided class

Which includes constructor instantiator if constructor is provided for the target class.

As we rely on parameter skip-constructor, users would expect that their constructors will never be invoked. Thus it makes sense to use org.objenesis.Objenesis#newInstance, as it:

Will create a new object without any constructor being called

# Conflicts: # bom-dependencies/pom.xml

piotrdytkowski · 2017-11-25T15:23:20Z

Review the changes, please. I had to make that last commit to retrigger the build, because it ... randomly failed. Thanks for the feedback, good spot with the ObjectInstantiator. Thanks 👍

orange-buffalo

@piotrdytkowski Thanks for the update!
Looks good to me.

* Add missing skipConstructor method to API (#218) * Fix checkstyle

garethahealy added enhancement needs-rebase work-required-to-accept-pr labels May 31, 2017

garethahealy mentioned this pull request May 31, 2017

Support for immutable types #216

Closed

piotrdytkowski force-pushed the master branch from 297186b to 22ad262 Compare June 27, 2017 19:49

piotrdytkowski pushed a commit to piotrdytkowski/dozer that referenced this pull request Jun 27, 2017

Added mapping immutable types DozerMapper#216 DozerMapper#218

22ad262

garethahealy requested changes Aug 6, 2017

View reviewed changes

piotrdytkowski force-pushed the master branch from 26a9613 to 9b70802 Compare November 15, 2017 21:53

piotrdytkowski pushed a commit to piotrdytkowski/dozer that referenced this pull request Nov 15, 2017

Added mapping immutable types DozerMapper#216 DozerMapper#218

9b70802

piotrdytkowski force-pushed the master branch from c18794b to 9b70802 Compare November 16, 2017 18:24

Added mapping immutable types DozerMapper#216 DozerMapper#218

88f07bd

piotrdytkowski force-pushed the master branch from 5a55c67 to 88f07bd Compare November 16, 2017 18:38

garethahealy reviewed Nov 18, 2017

View reviewed changes

garethahealy added needs-code-review and removed needs-rebase work-required-to-accept-pr labels Nov 18, 2017

orange-buffalo reviewed Nov 19, 2017

View reviewed changes

piotr.dytkowski added 6 commits November 25, 2017 12:17

Added objenesis to the dependency management

aa2f43b

Fixed forgotten renaming

39d3eac

More consistency in no-constructor strategy

4ca5619

Merge remote-tracking branch 'upstream/master'

3f99175

# Conflicts: # bom-dependencies/pom.xml

Added documentation for mapping immutable types.

6324f7e

Changed the docs topic

e32c04b

orange-buffalo reviewed Nov 26, 2017

View reviewed changes

garethahealy approved these changes Nov 29, 2017

View reviewed changes

garethahealy merged commit c87c5dc into DozerMapper:master Nov 29, 2017

CyberClems pushed a commit to CyberClems/dozer that referenced this pull request Mar 18, 2019

Add missing skipConstructor method to API (DozerMapper#218)

eb38902

garethahealy pushed a commit that referenced this pull request Apr 18, 2019

Add missing skipConstructor method to API (#218) (#748)

23eed8d

* Add missing skipConstructor method to API (#218) * Fix checkstyle

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mapping immutable types #216 #218

Mapping immutable types #216 #218

piotrdytkowski commented Dec 11, 2014

garethahealy commented Apr 11, 2017

garethahealy commented May 25, 2017

piotrdytkowski commented May 26, 2017

garethahealy commented Jun 22, 2017

piotrdytkowski commented Jun 27, 2017 •

edited

Loading

piotrdytkowski commented Jun 27, 2017 •

edited

Loading

garethahealy commented Jul 5, 2017

piotrdytkowski commented Jul 5, 2017 •

edited

Loading

piotrdytkowski commented Jul 11, 2017

garethahealy commented Jul 11, 2017

garethahealy Aug 6, 2017

garethahealy Aug 6, 2017

garethahealy Aug 6, 2017

garethahealy commented Aug 6, 2017

piotrdytkowski commented Nov 15, 2017

garethahealy commented Nov 16, 2017

piotrdytkowski commented Nov 16, 2017

piotrdytkowski commented Nov 16, 2017

dmitrySDK commented Nov 17, 2017 •

edited

Loading

piotrdytkowski commented Nov 17, 2017

garethahealy Nov 18, 2017

garethahealy commented Nov 18, 2017

garethahealy commented Nov 18, 2017

orange-buffalo left a comment

orange-buffalo Nov 19, 2017

orange-buffalo Nov 19, 2017

orange-buffalo Nov 19, 2017

piotrdytkowski commented Nov 25, 2017

orange-buffalo left a comment

Mapping immutable types #216 #218

Mapping immutable types #216 #218

Conversation

piotrdytkowski commented Dec 11, 2014

garethahealy commented Apr 11, 2017

garethahealy commented May 25, 2017

piotrdytkowski commented May 26, 2017

garethahealy commented Jun 22, 2017

piotrdytkowski commented Jun 27, 2017 • edited Loading

piotrdytkowski commented Jun 27, 2017 • edited Loading

garethahealy commented Jul 5, 2017

piotrdytkowski commented Jul 5, 2017 • edited Loading

piotrdytkowski commented Jul 11, 2017

garethahealy commented Jul 11, 2017

garethahealy Aug 6, 2017

Choose a reason for hiding this comment

garethahealy Aug 6, 2017

Choose a reason for hiding this comment

garethahealy Aug 6, 2017

Choose a reason for hiding this comment

garethahealy commented Aug 6, 2017

piotrdytkowski commented Nov 15, 2017

garethahealy commented Nov 16, 2017

piotrdytkowski commented Nov 16, 2017

piotrdytkowski commented Nov 16, 2017

dmitrySDK commented Nov 17, 2017 • edited Loading

piotrdytkowski commented Nov 17, 2017

garethahealy Nov 18, 2017

Choose a reason for hiding this comment

garethahealy commented Nov 18, 2017

garethahealy commented Nov 18, 2017

orange-buffalo left a comment

Choose a reason for hiding this comment

orange-buffalo Nov 19, 2017

Choose a reason for hiding this comment

orange-buffalo Nov 19, 2017

Choose a reason for hiding this comment

orange-buffalo Nov 19, 2017

Choose a reason for hiding this comment

piotrdytkowski commented Nov 25, 2017

orange-buffalo left a comment

Choose a reason for hiding this comment

piotrdytkowski commented Jun 27, 2017 •

edited

Loading

piotrdytkowski commented Jun 27, 2017 •

edited

Loading

piotrdytkowski commented Jul 5, 2017 •

edited

Loading

dmitrySDK commented Nov 17, 2017 •

edited

Loading