Skip to content

Commit

Permalink
fix: fix class and resource loading in maven plugin
Browse files Browse the repository at this point in the history 
Extracts logics from Flow mojos to separated task classes, loaded with a
custom class loader composed by project and plugin dependencies, with
the firsts having precedence on the others.
This make sure classes are always loaded from the same class loader,
preventing errors like having a class loaded by plugin class loader, but
one of its parent present only in project class loader (see #19616).
It also prevents retrieving resources from plugin dependency instead of
from same artifact defined in the project (see #19009).
This refactoring caches a ClassFinder per execution phase, so that it can be
reused by multiple goals configured to run on the same phase.
It also removes the need to instantiate a ClassFinder to checking for Hilla
classes, limiting the number or scans processed during the build.

Fixes #19616
Fixes #19009
Fixes #20385
  • Loading branch information
@mcollovati
mcollovati committed Nov 12, 2024
1 parent 80813cb commit 395f38c
Show file tree
Hide file tree
Showing 43 changed files with 3,030 additions and 810 deletions.
115 changes: 112 additions & 3 deletions ...flow-dev-bundle-plugin/src/main/java/com/vaadin/flow/plugin/maven/BuildDevBundleMojo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,17 @@

import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Function;
Expand All @@ -32,16 +37,25 @@
import org.apache.maven.artifact.Artifact;
import org.apache.maven.artifact.DependencyResolutionRequiredException;
import org.apache.maven.plugin.AbstractMojo;
import org.apache.maven.plugin.DefaultPluginRealmCache;
import org.apache.maven.plugin.MojoExecution;
import org.apache.maven.plugin.MojoFailureException;
import org.apache.maven.plugin.PluginRealmCache;
import org.apache.maven.plugin.descriptor.PluginDescriptor;
import org.apache.maven.plugins.annotations.Component;
import org.apache.maven.plugins.annotations.LifecyclePhase;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
import org.apache.maven.plugins.annotations.ResolutionScope;
import org.apache.maven.project.MavenProject;
import org.codehaus.plexus.classworlds.ClassWorld;
import org.codehaus.plexus.classworlds.realm.ClassRealm;
import org.codehaus.plexus.classworlds.realm.NoSuchRealmException;

import com.vaadin.flow.component.dependency.JavaScript;
import com.vaadin.flow.component.dependency.JsModule;
import com.vaadin.flow.component.dependency.NpmPackage;
import com.vaadin.flow.internal.ReflectTools;
import com.vaadin.flow.plugin.base.BuildFrontendUtil;
import com.vaadin.flow.plugin.base.PluginAdapterBase;
import com.vaadin.flow.plugin.base.PluginAdapterBuild;
Expand All @@ -53,7 +67,9 @@
import com.vaadin.flow.server.frontend.installer.NodeInstaller;
import com.vaadin.flow.server.frontend.installer.Platform;
import com.vaadin.flow.server.frontend.scanner.ClassFinder;
import com.vaadin.flow.server.scanner.ReflectionsClassFinder;
import com.vaadin.flow.theme.Theme;
import com.vaadin.flow.utils.FlowFileUtils;

import static com.vaadin.flow.server.Constants.VAADIN_SERVLET_RESOURCES;
import static com.vaadin.flow.server.Constants.VAADIN_WEBAPP_RESOURCES;
Expand Down Expand Up @@ -131,6 +147,12 @@ public class BuildDevBundleMojo extends AbstractMojo
@Parameter(defaultValue = "${project}", readonly = true, required = true)
MavenProject project;

@Component
MojoExecution mojoExecution;

@Component
PluginRealmCache pluginRealmCache;

/**
* The folder where `package.json` file is located. Default is project root
* dir.
Expand Down Expand Up @@ -175,17 +197,23 @@ public class BuildDevBundleMojo extends AbstractMojo
@Parameter(property = InitParameters.NPM_EXCLUDE_WEB_COMPONENTS, defaultValue = "false")
private boolean npmExcludeWebComponents;

private ClassFinder classFinder;

@Override
public void execute() throws MojoFailureException {
long start = System.nanoTime();

Runnable cleaner = augmentPluginClassloader();

try {
BuildFrontendUtil.runDevBuildNodeUpdater(this);
BuildFrontendUtil.removeBuildFile(this);
} catch (ExecutionFailedException | IOException
| URISyntaxException exception) {
throw new MojoFailureException(
"Could not execute build-dev-bundle goal", exception);
} finally {
cleaner.run();
}

long ms = (System.nanoTime() - start) / 1000000;
Expand Down Expand Up @@ -286,11 +314,43 @@ public File generatedTsFolder() {

@Override
public ClassFinder getClassFinder() {
if (classFinder == null) {
classFinder = createClassFinder(project);
}
return classFinder;
}

private static ClassFinder createClassFinder(MavenProject project) {
List<String> classpathElements = getClasspathElements(project);

return BuildFrontendUtil.getClassFinder(classpathElements);

URL[] urls = classpathElements.stream().distinct().map(File::new)
.map(FlowFileUtils::convertToUrl).toArray(URL[]::new);

// A custom class loader that reverts the order for resources lookup
// by first searching self and then delegating to the parent.
// This hack is required to prevent resources being loaded from the
// flow-server artifact the plugin depends on, giving priority to the
// flow-server version defined by the project.
// On the contrary, classes will be loaded first from the parent class
// loader, that should be the maven plugin class loader, but augmented
// with the project artifacts. This prevents class cast exceptions at
// runtime caused by classes loaded both from the plugin class loader
// and by the class loader used by Lookup (e.g.
// EndpointGeneratorTaskFactoryImpl from Hilla could not be cast to
// EndpointGeneratorTaskFactory because the interface is loaded by both
// the classloaders)
ClassLoader classLoader = new URLClassLoader(urls,
Thread.currentThread().getContextClassLoader()) {
@Override
public URL getResource(String name) {
URL resource = findResource(name);
if (resource == null) {
resource = super.getResource(name);
}
return resource;
}

};
return new ReflectionsClassFinder(classLoader, urls);
}

@Override
Expand Down Expand Up @@ -483,4 +543,53 @@ public boolean checkRuntimeDependency(String groupId, String artifactId,
public boolean isNpmExcludeWebComponents() {
return npmExcludeWebComponents;
}

/**
* Adds project artifacts to the plugin class loader to prevent class cast
* exceptions caused by plugin and Lookup loading separately the same
* classes.
*/
protected Runnable augmentPluginClassloader() {
PluginDescriptor pluginDescriptor = mojoExecution.getMojoDescriptor()
.getPluginDescriptor();
ClassRealm classRealm = pluginDescriptor.getClassRealm();
if (classRealm != null) {
List<String> classpathElements = getClasspathElements(project);
classpathElements.stream().distinct().map(File::new)
.map(FlowFileUtils::convertToUrl)
.forEach(classRealm::addURL);
// Plugin ClassRealm class loader is the same for all projects
// in the reactor. Flushing the cache to make sure a new realm
// is created for plugin every execution
return () -> resetPluginClassLoader(classRealm);
}
return () -> {
};
}

@SuppressWarnings("unchecked")
private void resetPluginClassLoader(ClassRealm classRealm) {
if (pluginRealmCache instanceof DefaultPluginRealmCache cache) {
logDebug("Removing plugin realm from cache");
try {
Field field = DefaultPluginRealmCache.class
.getDeclaredField("cache");
var cacheMap = (Map<PluginRealmCache.Key, PluginRealmCache.CacheRecord>) ReflectTools
.getJavaFieldValue(cache, field);
List<PluginRealmCache.Key> keys = cacheMap.entrySet().stream()
.filter(entry -> entry.getValue()
.getRealm() == classRealm)
.map(Map.Entry::getKey).toList();
keys.forEach(cacheMap::remove);
ClassWorld world = classRealm.getWorld();
world.disposeRealm(classRealm.getId());
mojoExecution.getMojoDescriptor().getPluginDescriptor()
.setClassRealm(null);
} catch (NoSuchFieldException | IllegalAccessException
| InvocationTargetException | NoSuchRealmException e) {
logWarn("Cannot reset plugin classloader", e);
}
}
}

}
30 changes: 29 additions & 1 deletion flow-plugins/flow-maven-plugin/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,34 @@
</plugin>
</plugins>
</pluginManagement>

<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-invoker-plugin</artifactId>
<version>3.6.1</version>
<configuration>
<skipInstallation>${skipTests}</skipInstallation>
<skipInvocation>${skipTests}</skipInvocation>
<localRepositoryPath>target/local-repo</localRepositoryPath>
<extraArtifacts>
<artifact>com.vaadin:flow-client:${project.version}</artifact>
</extraArtifacts>
<streamLogsOnFailures>true</streamLogsOnFailures>
<settingsFile>src/it/settings.xml</settingsFile>
<postBuildHookScript>verify</postBuildHookScript>
<addTestClassPath>true</addTestClassPath>
</configuration>
<executions>
<execution>
<id>integration-test</id>
<goals>
<goal>install</goal>
<goal>integration-test</goal>
<goal>verify</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
7 changes: 7 additions & 0 deletions flow-plugins/flow-maven-plugin/src/it/.gitignore
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
**/src/main/bundles
**/src/main/frontend/generated
**/src/main/frontend/index.html
**/package*.json
**/tsconfig.json
**/types.d.ts
**/vite.*.ts
18 changes: 18 additions & 0 deletions ...ns/flow-maven-plugin/src/it/appshellconfiguration-external-annotations/invoker.properties
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#
# Copyright 2000-2024 Vaadin Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not
# use this file except in compliance with the License. You may obtain a copy of
# the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations under
# the License.
#
invoker.goals=clean package
invoker.profiles.1=prepare-frontend-after-compile
invoker.profiles.2=build-frontend-full-dep-scan
118 changes: 118 additions & 0 deletions flow-plugins/flow-maven-plugin/src/it/appshellconfiguration-external-annotations/pom.xml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>com.vaadin.test.maven</groupId>
<artifactId>classfinder-lookup</artifactId>
<version>1.0</version>
<packaging>jar</packaging>

<description><![CDATA[
Tests that prepare-frontend does not fail when both plugin and ClassFinder loads
transitive classes from different classloaders.
In this test the plugin loads classes from spring-data-commons (RepositoryFactoryBeanSupport)
that implements ApplicationEventPublisherAware that is however in spring-context artifact.
Since spring-context is referenced only by the project, by default the plugin class loader
would not able to find ApplicationEventPublisherAware when loading RepositoryFactoryBeanSupport,
causing a ClassNotFoundException exception.
Usually this error happens when prepare-fronted is called after compilation, or when build-frontend is
configured with optimizeBundle=false, causing FullDependenciesScanner to be used.
Flow maven plugin must make sure that class loading works fine combining plugin and project dependencies.
]]></description>

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<maven.compiler.release>17</maven.compiler.release>
<maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
<maven.compiler.target>${maven.compiler.release}</maven.compiler.target>
<maven.test.skip>true</maven.test.skip>

<flow.version>@project.version@</flow.version>
</properties>

<dependencies>
<dependency>
<groupId>com.vaadin</groupId>
<artifactId>flow-server</artifactId>
<version>${flow.version}</version>
</dependency>
<dependency>
<groupId>com.vaadin</groupId>
<artifactId>flow-client</artifactId>
<version>${flow.version}</version>
</dependency>
<dependency>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa</artifactId>
<version>3.3.4</version>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.13.0</version>
</plugin>
<plugin>
<groupId>com.vaadin</groupId>
<artifactId>flow-maven-plugin</artifactId>
<version>${flow.version}</version>
<dependencies>
<dependency>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-commons</artifactId>
<version>3.3.4</version>
</dependency>
</dependencies>
</plugin>
</plugins>
</build>

<profiles>
<profile>
<id>prepare-frontend-after-compile</id>
<build>
<plugins>
<plugin>
<groupId>com.vaadin</groupId>
<artifactId>flow-maven-plugin</artifactId>
<executions>
<execution>
<phase>compile</phase>
<goals>
<goal>prepare-frontend</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
<profile>
<id>build-frontend-full-dep-scan</id>
<build>
<plugins>
<plugin>
<groupId>com.vaadin</groupId>
<artifactId>flow-maven-plugin</artifactId>
<configuration>
<optimizeBundle>false</optimizeBundle>
</configuration>
<executions>
<execution>
<goals>
<goal>prepare-frontend</goal>
<goal>build-frontend</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>

</project>
Loading

0 comments on commit 395f38c

Please sign in to comment.