Skip to content
This repository has been archived by the owner on Nov 23, 2021. It is now read-only.

bean mapping #4

Open
hohwille opened this issue Jun 17, 2014 · 25 comments
Open

bean mapping #4

hohwille opened this issue Jun 17, 2014 · 25 comments

Comments

@hohwille
Copy link
Member

For bean mapping we should create a very small shim as module. We should also discuss if dozer is the right choice. Alternatives are mapstruct or orika. Dozer might be established but it has proven to be buggy.

@hohwille hohwille added this to the oasp:0.1.0 milestone Jun 17, 2014
@hohwille hohwille self-assigned this Jun 17, 2014
@maihacke
Copy link
Member

I vote for dozer, since were using it very succesfully for the last 6 years now. We had very few problems with bugs. For oasp we should decide for proven solutions and do not experiment.

@mmatczak
Copy link
Member

I'm not a dozer fan, but I think there is no serious alternative right now. What I did not like in dozer was the XML config, but now there is a Java config (http://dozer.sourceforge.net/documentation/apimappings.html) which looks very promising for me. How about going for the Java config?

@maihacke
Copy link
Member

were using both aproches. For one project we delevoped a custom adapter, which reads the config from excel and instruments dozer apis for configuration.

@hohwille
Copy link
Member Author

I vote for dozer, since were using it very succesfully for the last 6 years now.

I would not consider this as very successful. It has serious bugs and limits your usage. Orika looks way more promising to me. The coverage of a bean mapping framework is quite easy to test so we could give it a test and verify it. Until that I would go with dozer what we are currently using in the sample app.
But have a look at the 104 open issues such as:
DozerMapper/dozer#87
DozerMapper/dozer#81
DozerMapper/dozer#44
etc.
I do not consider this as a very healthy state and key to success.

@hohwille
Copy link
Member Author

I have created a tiny shim as wrapper for bean mapping. As implementation I delegate to dozer and can add some workarounds/fixes (e.g. mapping of null values, mapping of collections, etc.). I updated our sample application to use this new module and removed all direct references to dozer from the example. This already fixed some bugs. However, generic handling is still problematic.
Switching from dozer to e.g. orika can now be made very easy. Only the config and wrapper impl. has to be changed.

@mschmickler
Copy link

This wrapper is the right way. So we choose which features we expose to the application developer.
IMHO it is better to redesign the structure of the DTOs/CTOs and handle special cases at the application level than to use the most sophisticated mapping features. At least thats true for the straight development tasks.

@agudian
Copy link

agudian commented Jun 27, 2014

When it comes to bean-mappings, I just have to mention MapStruct (which I am involved in). MapStruct is an annotation processor that generates implementations for, in the simplest case, an interface that defines methods to map from Type A to B.
In our project (large scale), we just switched from Dozer to MapStruct for the following reasons:

  • to avoid certain dozer bugs, to better predict what happens in the bean-mapping (you can just look at the generated code, which looks like hand-written)
  • to be really type-safe. No more mapping a Car to a CustomerDto... 😉
  • last but not least, performance. Our App-Servers a bit weak on CPU power, so kicking out all the reflection within dozer helps us.

@hohwille
Copy link
Member Author

@agudian do you know orika as well? Maybe you can convince us that a code-generation approach is not causing pain in large scale projects. Orika has a IMHO smarter approach by generating the code on the fly. I have been using hibernate code generation, QueryDSL code generation, etc. in large scale projects and it is a pain! Developers update the code from version control, wait for everything to build and then figure out that they have to regenerate (maybe outside IDE with maven) and build again. This is to be avoided. How did you solve this?

@agudian
Copy link

agudian commented Jun 29, 2014

@agudian do you know orika as well? Maybe you can convince us that a code-generation approach is not causing pain in large scale projects. Orika has a IMHO smarter approach by generating the code on the fly.
I know Orika. As I see it, it has the same problem as dozer: you see problematic mapping declarations only at run-time, if you see them at all. MapStruct tells you at generation-time if there are unmapped properties in the target class (e.g. isbn vs iSBN). By default, an error is reported, but that is configurable.

For our large scale project, we chose to not perform the source generation (wsdl2java, xjc, JPA metamodel, MapStruct) on the fly. Main reason here was also the tideous IDE integration, which sometimes does the generation automatically, sometimes not, sometimes requires manual refreshes before or after the generation, etc... Based on your comment, you know what I'm talking about. 😉
So when ever we change an entity class or somehting like that (usually done by someone who generates a bulk of changes from EA), we run the code generation manually, either as mvn command, or with an external tool launcher within Eclipse. All generated code is committed to source-control, together with the changes to entites / dtos.

@timoe
Copy link

timoe commented Jul 22, 2014

Andreas is completly right. If you have big object graphs to map, Dozers impact on performance becomes measureable which is a showstopper for Dozer from my point of view.

As we faced very good experiences using MapStruct even in a large scaled project I vote for go on with mapstruct.

@hohwille
Copy link
Member Author

As I wrote, please provide unit tests and then we can also add a second implementation of the same interface for the moment we can keep the technical dependency as optional until decision is finalized.

@agudian
Copy link

agudian commented Jul 29, 2014

As I wrote, please provide unit tests and then we can also add a second implementation of the same interface for the moment we can keep the technical dependency as optional until decision is finalized.

That's not really how MapStruct works. You don't have a central mapper interface with just a method map, and thus not one central implementation that can map everything. Instead, we create specific mapper interfaces in the business components that contain methods such as toCarDto(Car entity) : CarDto - Mapstruct then only generates those implementations as plain Java-classes.

Besides the runtime-performance, this was one of the big sellings points for MapStruct in our project: you can map only between types for which you specified mapping methods. And the generator will give you warnings/erros in case some properties could not be mapped.

@hohwille
Copy link
Member Author

That's not really how MapStruct works. You don't have a central mapper interface with just a method map, and thus not one central implementation that can map everything.

  • For the test case approach this should not be a problem. You can simply create an implementation of the map method that delegates to the according and specific methods.
  • In the first place this does not appear as a pro argument to me for MapStruct. It makes the API more complicated. In many cases all you want to do is convert a list of transfer-objects to a list of entities and vice versa. The API we currently have is simple, generic and still efficient for this use-case. Orika and dozer both work this way. I would still tend for Orika as for me it seems to have the best of all worlds (efficiency, flexibility, type-safe, simplicity).

@hohwille hohwille modified the milestones: oasp:0.1.0, oasp:1.0.0 Aug 21, 2014
@hohwille hohwille removed their assignment Aug 22, 2014
@hohwille
Copy link
Member Author

So far nobody showed up with tests and an alternate implementation. We have do our first release with dozer. It is not perfect but works. Exchange can be done quite easily. However, I will keep the ticket open and just move to the next release.

@hohwille hohwille modified the milestones: backlog, oasp:1.0.0 Dec 15, 2014
@hohwille
Copy link
Member Author

hohwille commented Feb 5, 2015

@oelsabba if you are looking for a way to run the same tests once with dozer and once with orika have a look here for how this can be done with JUnit:
https://github.com/m-m-m/mmm/blob/master/mmm-client/mmm-client-ui/mmm-client-ui-widget/mmm-client-ui-widget-impl-test/src/test/java/net/sf/mmm/client/ui/impl/test/AbstractUiTest.java

Just use something like this in your case:

@parameters
public static Collection<Object[]> parameters() {
return Arrays.asList(new Object[] { "dozer" }, new Object[] { "orika" });
}

oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 12, 2015
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 12, 2015
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 12, 2015
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 17, 2015
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 17, 2015
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 17, 2015
…st.java to run separate test methods for Orika and Dozer
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 18, 2015
…test once with OrikaBeanMapper and once with DozerBeanMapper
oelsabba added a commit to oelsabba/oasp4j that referenced this issue Feb 18, 2015
@hohwille
Copy link
Member Author

I have meanwhile created a test for the bean mapper that is running on Dozer. Next we will integrate (parts) of the PR from @oelsabba and provide an Orika implementation as well. Also we need to extend the test as it is only testing one use-case.

@agudian
Copy link

agudian commented Jun 10, 2015

Funny, yesterday I forked oasp4j for the first time to check what I might have to do to demonstrate the usage of MapStruct in the project...

As MapStruct follows a totally different approach as dozer or orika, it can't be used as a drop-in replacement of them and it especially makes no sense in hiding it behind a common BeanMapper interface. So I'd have to change the callers of the mapper... or at least of some of them, to demonstrate the differences (the ObjectMapper can be used in parallel to the MapStruct-generated mappers).

Would oasp in general be interested in such a demonstration?

hohwille added a commit that referenced this issue Jun 10, 2015
@hohwille
Copy link
Member Author

Would oasp in general be interested in such a demonstration?

Our course it is interesting. Maybe you can convince people easier. However, I would suggest that you keep your effort low to show a single usage on a fork so in case it does not get merged you wont feel bad.
I still think that it is a lot simpler to have just one simple component to inject and to deal with. It can still be type-safe with our approach by providing a generic method variant. I can add that in the next days, when I will find the time...
Maybe you can convince with performance:
https://twitter.com/joao_b_reis/status/559780053979250688

That is at least why I want to go away from dozer.

@hohwille
Copy link
Member Author

So when ever we change an entity class or somehting like that (usually done by someone who generates a bulk of changes from EA), we run the code generation manually, either as mvn command, or with an external tool launcher within Eclipse. All generated code is committed to source-control, together with the changes to entites / dtos.

@agudian then MapStruct in the end is IMHO nothing but a code-generator. We already have CobiGen for that purpose. We can give @may-bee the challenge to write templates for CobiGen that do the same as MapStruct does :)

@agudian
Copy link

agudian commented Jun 10, 2015

@agudian then MapStruct in the end is IMHO nothing but a code-generator.

Exactly, MapStruct is a code-generator. An annotation-processort, that generates code at compile-time. The comment of mine above with the manually triggered code generation is almost a year old.
In the meantime, we switched to on-the-fly generation during the compilation (as it is intended), and it works great - both in javac in maven builds and in incremental builds within Eclipse or the ad-hoc builds in IntelliJ - IDE integration for annotation-processors works quite well nowadays. You get immediate feedback if some new or changed bean properties can't be mapped. That's were both the generic mapping frameworks like dozer/orika and manually triggered generators can't compete. 😉

I could write a lot more on how we use MapStruct in our projects, for what we use those mappers and so on... But I'll see that I add a MapStruct mapper to one of the components as a showcase in a new PR first, that might illustrate things a bit better.

agudian added a commit to agudian/oasp4j that referenced this issue Jun 10, 2015
agudian added a commit to agudian/oasp4j that referenced this issue Jun 10, 2015
hohwille added a commit that referenced this issue Jun 11, 2015
hohwille added a commit that referenced this issue Jun 12, 2015
agudian added a commit to agudian/oasp4j that referenced this issue Jun 24, 2015
@hohwille
Copy link
Member Author

See #319

amarinso pushed a commit to amarinso/oasp4j that referenced this issue Feb 9, 2016
Force locale to en_EN in order to make the tests pass
@hohwille
Copy link
Member Author

@amarinso
Copy link
Member

amarinso commented May 3, 2017

Just revisiting some issues for research... I noticed that the last update of Dozer 5.5.1 is from 22/04/2014... that is, we are relying on a 3 years old project...

Do we have better experience now with alternatives and can propose a change for OASP4J?

@ivanderk
Copy link
Member

ivanderk commented May 3, 2017

Orika seems to be well maintained. https://mvnrepository.com/artifact/ma.glasnost.orika/orika-core
But on Github people seem to be very active working on a 6.0 https://github.com/DozerMapper/dozer

@maybeec
Copy link
Member

maybeec commented May 3, 2017

Btw. there seems to be fresh air going through the old lines of code of dozer: https://github.com/DozerMapper/dozer/pulse
My bug fixe has been merged some weeks ago. We should keep our eyes open as there might be a relese coming up soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants