oasp / oasp4j

The Open Application Standard Platform for Java
Apache License 2.0
60 stars 303 forks source link

bean mapping #4

Open hohwille opened 10 years ago

hohwille commented 10 years ago

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.

maihacke commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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: https://github.com/DozerMapper/dozer/issues/87 https://github.com/DozerMapper/dozer/issues/81 https://github.com/DozerMapper/dozer/issues/44 etc. I do not consider this as a very healthy state and key to success.

hohwille commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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:

hohwille commented 10 years ago

@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 commented 10 years ago

@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. :wink: 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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 9 years ago

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 commented 9 years ago

@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" }); }

hohwille commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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. :wink:

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.

hohwille commented 9 years ago

See #319

hohwille commented 8 years ago

https://www.frank-rahn.de/java-bean-mapper

amarinso commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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.