Add simple Quarkus demo application

[chris922]

As already mentioned in @gunnarmorling thread on Twitter I created a small example using MapStruct together with Quarkus.

This here is the result. I thought it might be good to have this in our examples repo, even if there is really nothing special.

I also added a README containing a few more information.

The example itself just ramps up a small web-app that uses a PersonService that returns a Person object, mapping it to a PersonDto and sends the result to the caller.

Should we add this here? WDYT?

[agudian]

Looks good! I think it’s definitely worth adding.

Two things, though: please check the year in the license headers and configure the component model in the @Mapping annotation - that’s the preferred way, I think and it might be a bit easier to find for folks checking the example.

I like the readme! Strange that Quarkus in dev mode doesn’t honor annotation processors, though. It still recompiles other changed classes, right?

[chris922]

Thanks @agudian ! I just copied the license headers without checking them.. maybe it is worth to rewrite all headers in this repository at once like @filiphr did in the main mapstruct repo. But this shouldn't be done within this PR and therefore I think it might better to not change the header so that a search & replace is easier?

Gotcha! When I added the compile argument I knew that someone will notice this... 🤣
I personally like the compile argument much more as this is a global configuration and it shouldn't be possible to miss such an important config. But as this here is a really small example I can change this 😃

By the way: It looks like Quarkus is not running with JDK12. @filiphr : Is it somehow possible to disable tests for JDK12?

[chris922]

@agudian : CDI is now set in an own config. I haven't changed the license header as said in my previous post to make a batch-update easier.

Regarding dev-mode: I tried a few more things.. reloading of e. g. the service works, but even if I make a change directly at the mapper interface it will be reloaded according to the logging, but the generated class will not be updated. 😞

[gunnarmorling]

Let me summon @gsmet on this one; Guillaume, what's the Quarkus way to enable a reflective usage like this with an extension in a generic way? Some background: while MapStruct generally avoids the usage of reflection, there's a single place where it's optionally used when not working with CDI or Spring: obtaining a mapper implementation. The calling code in user apps looks like this:

CustomerMapper cm = Mappers.getInstance(CustomerMapper.class);

MapStruct's getInstance() method essentially does return Class.forName(passedClass.getName() + ".Impl").newInstance();

IIUC, we could enable this usage by defining a Quarkus extension in a generic way.

[chris922]

I played a bit around with a few ideas I had to find a generic way.. the only idea left that looks promising is to extend our processor to create something like a list of mappers (e. g. just line-by-line in a new file META-INF/mapstruct.mappers), access this list in a Feature and register all these classes.

I was working on a PoC for this and it seems to work. Maybe it is also possible to include this Feature in the MapStruct library having the graalvm/substratevm dependencies marked as optional or provided. As this class will not be accessed outside from SubstrateVM it shouldn't make any troubles if it is just available somewhere in the JAR.

Another idea I had was to use the ServiceLoader as I thought the META-INF/services/* files are maybe already analyzed by the SubstrateVM... but unfortunately this was not the case. Even more worse: When these service files will be used to register the classes for reflection during native-image build it is additionally required to add a property during native-image build to include these service files in the image. Otherwise the ServiceLoader is not able to find them.

The other way described on GraalVM pages to enable reflection would be to create a JSON containing all the mappers and add a build property to load this JSON. But I don't like to have the need to modify build parameters, so I prefer the AutomaticFeature way.

Regarding the Quarkus extension: Writing such an extension would probably the best way to support Quarkus. But maybe we can even find a way to support the GraalVM native-image in general?

[gsmet]

So the idea with Quarkus is to just produce ReflectiveClassBuildItem and Quarkus will do the heavy lifting for you.

Another nice thing is that Jandex will index the application classes (and potentially additional jars if they are properly defined) so you could search for all the implementations of a given interface and register them all automatically.

All in all I don’t think it would take long to create a Quarkus extension to cover that.

[gsmet]

I added a few minor comments on the pom, mostly due to issues on our current template. They are fixed for the next version.

[gunnarmorling]

@gsmet, is this expected that annotation processors aren't re-run in dev mode? Or should we file an issue against Quarkus?

[gsmet]

I think it's expected they aren't. Not sure if it would be feasible to run them. Create an issue and see what Stuart has to say about it.

[filiphr]

I see big discussion around this PR. It is nice to see that. Just my 2 cents by looking at the config. Is it possible that Quarkus doesn't even see the processor as it is in the maven-compiler via the annotationProcessorPaths? @chris922 have you tried putting the processor on the provided scope? (I don't know how Quarkus works, just something from the top of my head 😄)

[gsmet]

I don't think we take them into account at all in Quarkus dev mode. That's why I asked Gunnar to create an issue.

[chris922]

Good idea @filiphr ! I tested it and it partially works. 😃
When I just add the processor as a regular provided dependency Quarkus is picking this up in dev-mode. When I make a change to the mapper and reload the page the implementation will be regenerated. As I said partially it works, in case I just change e. g. the DTO the mapper implementation will not be regenerated (I think it would be really complex to solve this issue as well as Quarkus needs to notice that the mapper depends on this class and must compile the mapper as soon as the DTO changes)

I will mention this in quarkusio/quarkus#1502

[chris922]

@gsmet : First of all - thanks for jumping in!

I just found the RegisterForReflection annotation, what if we add this to all generated classes that should be available for Class.forName(...) and calling the constructor afterwards - is this already be sufficient as a first step?
Of course, writing an extension is much better and more powerful. Thats imho definitely something we should do as well in case the interest for Quarkus will still be there in the future.

[gsmet]

Yes @RegisterForReflection should work (as long as the classes are part of the application classpath that gets indexed by Jandex).

[filiphr]

Awesome for working on this @chris922. Thanks to @gsmet for stepping up and helping out with the review as well.

I also agree that it is worth adding this to our examples.

By the way: It looks like Quarkus is not running with JDK12. @filiphr : Is it somehow possible to disable tests for JDK12?

You can disable the tests by adding a profile that would get activated and skip the tests in Java 12 with:

<profile>
    <id>java12+</id>
    <activation>
        <jdk>[12,)</jdk>
    </activation>
    <properties>
        <maven.test.skip>true</maven.test.skip>
    </properties>
</profile>

@chris922 do you want to open an issue for the Java 12 problem with the Quarkus team?

---

Just for linking the problem with the dev mode, @gunnarmorling raised quarkusio/quarkus#1502 in the quarkus project.

[chris922]

Thanks @filiphr - I created a new issue quarkus/issues#1534 and disabled the tests and quarkus plugin for jdk >= 12.

EDIT: I missed to update the README.md, will do this asap.
EDIT2: README updated, so I think there is nothing left open for this PR.

[filiphr]

@chris922 I completely missed the edits inn your last commit (no emails are sent for an edit 😄).

If the PR is ready then feel free to merge when you have time

[chris922]

Thank you very much @filiphr

I totally forgot about this topic as well ... I just updated the Quarkus version to 0.15.0 and aligned my changes with the latest one from master.

I will proceed with merging this PR