fix: fix class and resource loading in maven plugin

mcollovati commented 3 days ago

Description

Run Flow mojos using an isolated class loader that includes both project and plugin dependencies, with project dependencies taking precedence. This ensures that classes are always loaded from the same class loader at runtime, preventing errors where a class might be loaded by the plugin's class loader while one of its parent classes is only available in the project’s class loader (see #19616).

Additionally, this approach prevents the retrieval of resources from plugin dependencies when the same artifact is defined within the project (see #19009).

This refactoring also introduces caching for ClassFinder instances per execution phase, allowing multiple goals configured for the same phase to reuse the same ClassFinder. It also removes the need to instantiate a ClassFinder solely for Hilla class checks, reducing the number of scans performed during the build.

Fixes #19616 Fixes #19009 Fixes #20385

Type of change

[x] Bugfix
[ ] Feature

Checklist

[x] I have read the contribution guide: https://vaadin.com/docs/latest/guide/contributing/overview/
[x] I have added a description following the guideline.
[x] The issue is created in the corresponding repository and I have referenced it.
[x] I have added tests to ensure my change is effective and works as intended.
[x] New and existing tests are passing locally with my change.
[ ] I have performed self-review and corrected misspellings.

Additional for `Feature` type of change

[ ] Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

mcollovati commented 3 days ago

If this PR gets merged, the only change in Hilla should be fixing mock configuration in hilla-maven-plugin tests

github-actions[bot] commented 3 days ago

Test Results

1 147 files +1 1 147 suites +1 1h 32m 22s ⏱️ - 1m 11s 7 493 tests +4 7 443 ✅ +4 50 💤 ±0 0 ❌ ±0 7 864 runs +5 7 804 ✅ +5 60 💤 ±0 0 ❌ ±0

Results for commit de63c040. ± Comparison against base commit 05f03774.

:recycle: This comment has been updated with latest results.

sonarcloud[bot] commented 1 day ago

Quality Gate passed

Issues
14 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mcollovati commented 1 day ago

Could Reflector and also some methods in BuildDevBundleMojo and FlowModeAbstractMojo go in shared flow-plugin-base module? There's already dependency to flow-plugin-base and both mojo's seem to use classes from there. See BuildFrontendUtil for example.

I was thinking the same, but followed the current solution of copy/pasting code between mojos. I'll try to refactor as you suggest.

mcollovati commented 1 day ago

Looking at the code again, what blocks me from moving Reflector to the common module is that it references Maven API but the module is used also by gradle plugin.

vaadin / flow