Add autoconfiguration for Spring MVC

[dsyer]

Provisional structure of autoconfiguration and use in one of the sample. Maybe need to add more conditionals later. Also need to add WebFlux, if this is the right approach.

I had to hack the module-info.java a lot, and I'm a JPMS moron, so please help me understand any changes needed there.

[dsyer]

I don't think I like the SpringTemplateConfig. I kind of get the motivation, but I don't want to have to remember to add @Component to all my renderers and apparently JStachio doesn't work without that. Did I miss something? I guess we could make the SpringJStachio conditional on actually finding some of those beans.

[agentgt]

I think we can just use the JStache annotation directly for lookup for auto configure.

Also take a peak at JStachCatalog

The original spring stuff was more to showcase anything is possible to make sure the API is right.

I’ll pull your changes to my own repo later today and give feedback.

[dsyer]

Oops. Your new test now fails with this autoconfiguration. I reverted to before the test was added.

[agentgt]

Most of the changes just require you go and change all requires transitive spring.* to requires spring.*

Then every downstream module (like the example projects) will still need requires spring.*. The rule of thumb is to never requires transitive on any automatic module (no module-info.class).

Some day Spring will have true modules and your way would have worked. So you had the right intention.

[dsyer]

I fixed it like you said. But I don't understand because it did actually work.

[agentgt]

I fixed it like you said. But I don't understand because it did actually work.

The transitive (of automatic modules) will appear to work in most configurations but in a true modular one it will not.

It is kind of complicated why that is.

I'll find some more docs as to why that is the case later.

[agentgt]

@dsyer see this:

https://stackoverflow.com/questions/46750408/why-does-javac-complain-about-named-automatic-modules

I will try to find more.

As a bonus albeit tedious doing it the non transitive way removes a compiler warning.

[agentgt]

When you get a chance can you rebase on main?

[dsyer]

When I do that the new integration test fails. I don't understand why. Something to do with the HelloModelAndView? I can push it to my fork if you want.

[agentgt]

Don’t worry I’ll take a look shortly and either comment a fix or do a PR to your branch

[agentgt]

I can't figure out why but the auto configure is not picking it up.

I even converted the project to an Automatic module and it still never instantiates the io.jstach.opt.spring.boot.webmvc.JStachioWebMvcAutoConfiguration.

Tried both spring:boot-run and junit.

I even tried the old spring.factories way.

EDIT also tried @AutoConfigure instead of @Configure.

[dsyer]

OK. I’ll figure it out from there.

[agentgt]

I think I found the problem. It is because the example application is a modularized.

I need to figure out a way to replace Spring Boot's Classloader used for autoconfiguration loading.

See org.springframework.boot.context.annotation.ImportCandidates

[agentgt]

You had the wrong name for the imports file.

You had:

org.springframework.boot.AutoConfiguration.imports

It should be:

org.springframework.boot.autoconfigure.AutoConfiguration

I'm ashamed how long it took me to figure that out. I missed it several times and thought it was correct and thought it was a classloader issue.

[dsyer]

Grr. Sorry. Thanks for spotting that. I would have got there eventually. Rebased and squashed with correct file name.

I still don't like the "every template is a @Component" pattern. Could we make that conditional on at least some components being templates? Or fallback to the default template finder if something isn't found that way?

[agentgt]

Yes I was working on that with your branch.

[agentgt]

@dsyer pull my PR dsyer#2 on to your mvc-auto branch

[agentgt]

@dsyer Maybe we should make a temporary mvc-auto branch directly on the jstachio/jstachio so that we can collaborate (no rebasing till we are done)?

For now I have been pulling your mvc-auto and rebasing on it so in theory if you pull now (dsyer#2) it should be a simple fast-forward.

EDIT: I have not yet figured out the best way to combine extensions from the serviceloader and spring registered ones or if I should even do that.

The templates are an exception because there is not a lot of configuration that can be done.

[dsyer]

With the templates it looks like you are loading from the service loader and registering each one as a singleton? Why? What if they were actually already @Component (then they would be registered twice)? I can't see any code that needs them to be beans. Can't they just be used as a fallback?

Unrelated: JStachioAutoConfiguration is now empty. We might as well put all the code from SpringTemplateConfig in there and delete the latter? Or am I missing something?

[agentgt]

Unrelated: JStachioAutoConfiguration is now empty. We might as well put all the code from SpringTemplateConfig in there and delete the latter? Or am I missing something?

Yes we are on the same page there. I left it to make it easier to diff for the time being.

With the templates it looks like you are loading from the service loader and registering each one as a singleton? Why? What if they were actually already @component (then they would be registered twice)? I can't see any code that needs them to be beans. Can't they just be used as a fallback?

HelloController wires in a template directly.

I remove the @Component JStacheInterfaces. The hope is to make this as easy as possible since it is a boot starter so this was an opinionated choice on my part as I have strong doubts most "booters" will have the patience for JStacheInterfaces.

The only time one would want to manually register templates is to change the TemplateConfig. I'm pushing another change in mvc-auto-fix to show that.

[agentgt]

@dsyer When you get a chance see the update to my PR: dsyer#2

[dsyer]

Merged into my fork.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
1.7% Duplication

[agentgt]

@dsyer I went ahead and merge into main. Even though it probably is not completely done I think it is safe enough and I think some changes are arguably bug fixes (defaultTemplateFinder supportsType comes mind).

Feel free to pull main and submit another PR against it if you like.

I'm on vacation next week so my turn around time will be longer.

Thanks for working on this with me!