feat!(generator): load endpoints from Spring components and stop using Maven/Gradle at each generation #2616
Conversation
cromoteca
force-pushed
the
feat/avoid-running-maven
branch
4 times, most recently
from
30c2e88
to
236efd5
Compare
cromoteca
force-pushed
the
feat/avoid-running-maven
branch
from
9d4988c
to
a917323
Compare
cromoteca
force-pushed
the
feat/avoid-running-maven
branch
from
d7e3b47
to
4023bde
Compare
|
cromoteca
changed the title
There was a problem hiding this comment.
Would you consider the introduction of an interface to make the endpoint search independent of Spring AOT?
Something like
@FunctionalInterface
public interface EndpointProvider {
List<Class<?>> findEndpoints(EngineConfiguration configuration) throws ExecutionFailedException;
}
and making AotEndpointFinder implement it.
The EngineGenerateMojo mojo would then have a @Paramter annotated EngineConfiguration.EndpointProvider field, defaulting to AotEndpointFinder and would simply do var endpoints = endpointProvider.findEndpoints(conf);.
This should allow implementing custom endpoint finders not based on Spring by configuring the plugin; for example the implementation can search a Jandex index.
<endpointProvider implementation="org.vaadin.test.MyEndpointPRovider"></endpointProvider>
Actually, I don't know how to make it work in Gradle, but I suppose this should be possible.
There was a problem hiding this comment.
Another question: why does it need to run SpringApplicationAotProcessor to find endpoints? Couldn't the scan be done with Flow ClassFinder?
There was a problem hiding this comment.
I have the general feeling that making Hilla depend more on Spring will actually make easier to isolate Spring dependency. For example, at the moment Spring is heavily used in the connector, but not in the generator.
The whole point of this PR is to let Spring decide which endpoints are available. A single provider must be trusted both in the code generation and in the controller which runs in the application. When they will be coherent, it will be easier to swap the "source of truth".
Concerning the ClassFinder, while I don't know it well, I guess it's just another way to walk the classpath searching for annotated classes. Correct me if I'm wrong, but that's what I'm trying to avoid here: finding too many endpoints, that are in the classpath, but not loaded by Spring.
There was a problem hiding this comment.
Got it. Thanks for clarification.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2616 +/- ##
=======================================
Coverage 92.64% 92.64%
=======================================
Files 83 83
Lines 2842 2842
Branches 739 739
=======================================
Hits 2633 2633
Misses 158 158
Partials 51 51
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
Removes the need of calling Maven (or Gradle) at each code generation during development.
Endpoints are now searched between the available Spring Beans. When building for production mode, since the application is not running, some code provided by Spring is run to be able to get a list of beans as if the application was running.
The generator configuration through Maven is no longer possible, since the application can be run directly as a main class. The idea is to provide it as a singleton, which should become a Spring bean. At the moment, it's just an ordinary class and cannot be configured externally, but since that's internal code, the change can happen in future, when some configuration means has been chosen.
Gradle support is work in progress, not tested (doesn't even compile, actually).
Many tests are touched by these changes and some no longer work, so further fixes are needed, to the tests themselves to accommodate for the new workflow, or to the code if those tests should keep passing.
The new
annotationspackage should probably be renamed as it also contains a type.