Skip to content

Added draft v1 of jaxb contract first#532

Merged
garethahealy merged 2 commits intoDozerMapper:masterfrom
garethahealy:jaxb
Mar 27, 2018
Merged

Added draft v1 of jaxb contract first#532
garethahealy merged 2 commits intoDozerMapper:masterfrom
garethahealy:jaxb

Conversation

@garethahealy
Copy link
Collaborator

This PR:

  • Adds a JAXB based model, built originally from the schema.
  • Model can also be used via a fluent API, i.e.: .withThingToChange(thing)
  • This feature is not enabled by default, but can be via a property
  • The reason for the elengine package model is due to not being able to insert a ELEngine instance when JAXB is parsing.

This feature wont be abled by default for a few releases, just to make sure nothing has been broken.

CC @orange-buffalo for code review.

Copy link
Contributor

@orange-buffalo orange-buffalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@garethahealy I have preliminary checked the code, looks very good to me. There is one important comment about Lombok. If you could have a look and provide your feedback, I would then continue the review.


DozerBeanMapper mapper = (DozerBeanMapper)factory.getObject();
List<?> files = mapper.getMappingFiles();
//List<?> files = mapper.getMappingFiles();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to handle this commented code to not forget about it. Maybe some todo could help here, if changing the test is not feasible in the first iteration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why that was commented out. Have uncommented and test still works.

<id>schemagen</id>
<phase>generate-resources</phase>
<goals>
<goal>schemagen</goal>
Copy link
Contributor

@orange-buffalo orange-buffalo Nov 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember last time we had a discussion about using Lombok in model classes, to remove those boilerplate getters, setters and toStrings, but after adding Lombok the build was failing. I created a mojohaus/jaxb2-maven-plugin#88 to address this, but did not get any response yet.

But, I was able to add Lombok and build the project with previous major version of the jaxb2 plugin (they rewrote a lot in 2x). In my opinion, clean code in core is more important than using the latest maven plugin version, thus I would recommend to apply this solution.

In order to build with lombok, I changed the jaxb2-maven-pluginversion to 1.6 and its execution configuration to the following:

                     <execution>
                        <id>schemagen</id>
                        <phase>generate-resources</phase>
                        <goals>
                            <goal>schemagen</goal>
                        </goals>
                        <configuration>
                            <workDirectory>${project.build.directory}/generated-resources/schemagen</workDirectory>
                            <includes>
                                org/dozer/builder/model/jaxb/**
                            </includes>
                            <transformSchemas>
                                <transformSchema>
                                    <uri>http://dozermapper.github.io/schema/bean-mapping</uri>
                                    <toFile>bean-mapping.xsd</toFile>
                                </transformSchema>
                            </transformSchemas>
                        </configuration>
                    </execution>
                    <execution>
                        <id>xjc</id>
                        <phase>generate-test-sources</phase>
                        <goals>
                            <goal>xjc</goal>
                        </goals>
                        <configuration>
                            <schemaDirectory>
                                ${project.basedir}/src/test/xsd/jaxb
                            </schemaDirectory>
                            <packageName>org.dozer.vo.jaxb.employee</packageName>
                            <outputDirectory>target/jaxb-sources</outputDirectory>
                        </configuration>
                    </execution>

After that the plugin generated the XSD from classes with Lombok annotations applied.

Copy link
Collaborator Author

@garethahealy garethahealy Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. have gave the model classes a lombok once over

dozerClass.setMapGetMethod(mapGetMethod);
dozerClass.setMapSetMethod(mapSetMethod);
dozerClass.setCreateMethod(createMethod);
dozerClass.setMapNull(mapNull == null ? null : mapNull);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could just pass the mapNull as the result of ternary operator will be always equal to the mapNull?

*/
@Override
public List<MappingFileData> build(BeanContainer beanContainer, DestBeanCreator destBeanCreator, PropertyDescriptorFactory propertyDescriptorFactory) {
List<MappingFileData> answer = new ArrayList<MappingFileData>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could cleanup a bit here and use the diamond operator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@garethahealy garethahealy mentioned this pull request Dec 9, 2017
@garethahealy
Copy link
Collaborator Author

@orange-buffalo ; any further comments regarding review?

Copy link
Contributor

@orange-buffalo orange-buffalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@garethahealy garethahealy merged commit 9d6e4bf into DozerMapper:master Mar 27, 2018
@garethahealy garethahealy deleted the jaxb branch March 27, 2018 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants