jooby
jooby copied to clipboard
jooby-project
java modules: modularize jooby modules
@imeszaros Do you know Guice well? I had to upgrade javax.inject to jakarta.inject-api and put in a hack that will probably not work for all cases (see #2631).
Alright the next change is going to be massive. I have to replace javax.annotation aka JSR305 (defunct).
Obviously we need to choose an annotation that is Kotlin friendly. Here is the list on that: https://kotlinlang.org/docs/java-interop.html#nullability-annotations
My personal preference would be JSpecify but it is only 0.2. It would also require the most work as I have to make a package-info.java and remove all the @Nonnull.
I think the best replacement for now is:
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
<version>4.7.2</version>
</dependency>
+1 for spotbugs. Ambition behind JSpecify is great but they state clearly not to depend on it yet.. Who knows, maybe they'll release 1.0 before the Jooby 3 release ;)
OK I'm done for now with getting Jooby core as in the main jooby jar modularized.
The PR and branch is #2634. I'm sorry @jknack for all the PRs but I wanted to separate the changes out better. #2634 should be linear hopefully and combines the other PRs.
I plan on adding support to the ~~three~~ two module supported server implementations (utow is not possible at this time) which are netty and jetty.
Oh btw @jknack just by adding module-info I was able to find several places where you accidentally used JetBrains @NotNull. I went ahead and fixed those in the JSR 305 PR. You might want to fix those in 2.x as well.
Oh btw I'm not sure how to fix Sonar. Apparently the new annotations (@NonNull) give off more warnings mostly with builder like setters that return this.
Another thing I found while upgrading checkstyle (it has problems with module-info) is that io.jooby.ServerSentEmitter.KeepAlive is public. When you have a class inside an interface it is always public if the interface is public (regardless if the public modifier is there or not).
BTW on checkstyle. I personally think its value is pretty crappy. What you really want is autoformat on build for normal users and fail in github action if it would change a file. Like 99% of checkstyle's complaints are formatting.
Its really nice for other contributors as they don't have to worry about plugins and IDE settings.
I use the Eclipse maven formatter plugin but since this project uses Google Java format you can use Spotless instead.
N.B. The tests module (maven module) was using the same package as core jooby of io.jooby.
I moved those to io.jooby.test which was a PITA because the refactoring broke. https://github.com/jooby-project/jooby/pull/2634/commits/7cab9ffca0b294f1b3b135acc115651e68093408
Alright the build seems to be working.
Here is the module info for jooby:
https://github.com/agentgt/jooby/blob/jooby_module-info/jooby/src/main/java/module-info.java
The biggest issue is Kotlin and ASM. Those requires should be requires static which is kind of like Maven's <optional>true</optional>.
For some reason when I did that (requires static) the build would fail for Kotlin tests.
To fix the above problems and for better design I think we should split io.jooby:jooby (ie the main maven module) as follows:
- io.jooby:jooby-annotations - (io.jooby.annotations)
- io.jooby:jooby-core - (io.jooby)
- io.jooby:jooby-internal (io.jooby.internal)
Splitting the internal implementation (internal) will make it easier to fix some of the ASM/shade/kotlin issues I think.
@agentgt isn't a jakart.annotation replacement for javax.annotation? I prefer to not add spotbugs or jspecify.
@agentgt isn't a jakart.annotation replacement for javax.annotation? I prefer to not add spotbugs or jspecify.
@jknack I might have picked that but Kotlin does not work with it: https://kotlinlang.org/docs/java-interop.html#nullability-annotations
Besides those annotations are not recommended for null analysis. The future are TYPE_USE style annotations.
We are also in good company as I believe micronaut chose similar (spotbugs): https://github.com/micronaut-projects/micronaut-core/issues/1633
The spotbugs is the closest to the original findbugs aka jsr305 in terms of semantics. Unfortunately it is not TYPE_USE but it does have kotlin support.
The best really would be: jspecify, then checkerframework, then eclipse/intellij's annotations.
Yeah we can revisit the null annotations later. The TYPE_USE ones (jspecify, checkerframework, eclipse and intellij's annotations) generally require an annotation in package-info.java on every package. The default in those models once you annotate the package is that everything is not null unless it is marked (jspecify doesn't even have a @NotNull annotation).
Along with the additional work of package-info as well as TYPE_USE sometimes breaking annotation processors I also thought picking the ones above would have triggered sonar in doing full null analysis (e.g. checker or errorprone) so it is ironic that sadly my choice of spotbugs still caused analysis issues.
@jknack I'm not as familiar with Sonar so I'm not sure how to handle its new warnings.
@jknack Can we perhaps make another github issue(s) for doing the other Jooby extensions (aka the stuff in jooby-project/modules)?
@imeszaros I made a starter project called mod-starter.
(btw lots of the starter projects are kind of broken for me)
Here is the modules-info for it:
module starter.mod {
requires transitive io.jooby;
opens starter.mod to io.jooby;
}
n.b. here we cannot use the canonical provides MvcFactory with generatedModule as those classes are generated so we just let jooby have reflective access to the whole project (thats what opens does).
I have not tested with a full jlink build but it does appear to work when using the module classpath.
Ideally that starter would have a full jlink build.
@jknack we also need to modularize Handlebars for the full "jknack stack" (I like how that rhymes btw). I can do that later this week if you like.
Also apologies for flooding both of your inboxes. This was my "spike" / window to do the work. The good/bad news is I will have to wait like another week or two when I can contribute as I have some other obligations.
@agentgt agreed. let's create another issue for jooby extensions.
I would like to delete/remove starters, have no idea if they are useful I think jooby-console works better for getting starting and as you found out, they aren't maintained or updated frequently.
I appreciated if you can modularize handlebars too.
@agentgt @imeszaros Going to push a commit with module-info.java in all sub-modules where is possible.
For those where isn't... I'm going to add a Automatic-Module-Name to MANIFEST. For example: jooby-undertow
Also, I do prefer module name to be jooby, jooby.netty, etc than io.jooby, io.jooby.netty. Cool?
I think the canonical approach I have seen is to use the TLD.
I heard there was some talk of even sonatype (aka maven central) recommending that you use the group id.
That is why I chose io.jooby
most of them yes, they do. Spring is:
requires spring.core;
requires spring.beans;
requires spring.context;
I wonder why Spring did that. I have not used Spring 6 yet. I would be surprised if all the downstream Spring projects do that.
I don't blame you though. I have a new project and chose "io" ... and damn it is an expensive TLD now.... which reminds me I need to get some money your way for all the awesomeness.
On the other hand I'm not sure if having that TLD freedom really buys you anything as a consumer of the project still has to refactor if you do change it (ie io.jooby -> org.jooby).