Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Classes should not be loaded from the plugin #19010

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.Arrays;

import org.apache.maven.plugin.MojoFailureException;
Expand All @@ -39,6 +41,7 @@

import elemental.json.Json;
import elemental.json.JsonObject;

import static com.vaadin.flow.plugin.maven.BuildFrontendMojoTest.getPackageJson;
import static com.vaadin.flow.plugin.maven.BuildFrontendMojoTest.setProject;
import static com.vaadin.flow.server.Constants.PACKAGE_JSON;
Expand Down Expand Up @@ -300,8 +303,16 @@ private void enableHilla() throws IOException {
// detected as Hilla project with endpoints.
Files.createDirectories(Paths.get(projectBase.toString(), "target")
.resolve("test-classes/com/vaadin/hilla"));
Files.createFile(Paths.get(projectBase.toString(), "target").resolve(
"test-classes/com/vaadin/hilla/EndpointController.class"));
Path tempHillaClass = Files
.createFile(Paths.get(projectBase.toString(), "target").resolve(
"test-classes/com/vaadin/hilla/EndpointController.class"));
Path testModuleHillaClass = new File(
System.getProperty("user.dir", ".")).toPath()
.resolve(Path.of("target", "test-classes", "com", "vaadin",
"hilla", "EndpointController.class"));
Files.copy(testModuleHillaClass, tempHillaClass,
StandardCopyOption.REPLACE_EXISTING);

Files.createDirectories(
Paths.get(projectBase.toString(), "src/main/frontend"));
Files.createFile(Paths.get(projectBase.toString(), "src/main/frontend")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.vaadin.flow.plugin.maven;

import java.io.File;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;

Expand All @@ -22,10 +24,12 @@
import com.vaadin.flow.server.frontend.EndpointGeneratorTaskFactory;
import com.vaadin.flow.server.frontend.FrontendTools;
import com.vaadin.flow.server.frontend.scanner.ClassFinder;
import com.vaadin.flow.theme.AbstractTheme;

import static com.vaadin.flow.server.Constants.PACKAGE_JSON;
import static com.vaadin.flow.server.Constants.VAADIN_SERVLET_RESOURCES;
import static com.vaadin.flow.server.frontend.FrontendUtils.DEFAULT_FRONTEND_DIR;
import static org.mockito.Mockito.when;

public class GenerateNpmBOMMojoTest {

Expand Down Expand Up @@ -109,6 +113,16 @@ public void setUp() throws Exception {
.lookup(ClassFinder.class);
return lookup;
}).when(mojo).createLookup(Mockito.any(ClassFinder.class));
when(project.getRuntimeClasspathElements())
.thenReturn(projectClassPath());

}

private static List<String> projectClassPath() throws URISyntaxException {
List<String> classPaths = new ArrayList<>();
classPaths.add(AbstractTheme.class.getProtectionDomain().getCodeSource()
.getLocation().toURI().getPath());
return classPaths;
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public class ReflectionsClassFinder implements ClassFinder {
* the list of urls for finding classes.
*/
public ReflectionsClassFinder(URL... urls) {
classLoader = new URLClassLoader(urls,
Thread.currentThread().getContextClassLoader());
ClassLoader parent = ClassLoader.getPlatformClassLoader();
classLoader = new URLClassLoader(urls, parent);
ConfigurationBuilder configurationBuilder = new ConfigurationBuilder()
.addClassLoaders(classLoader).setExpandSuperTypes(false)
.addUrls(urls);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,17 @@ public class ReflectionsClassFinderTest {
+ "import com.vaadin.flow.component.Component;\n" + "\n"
+ "@NpmPackage(value = \"@vaadin/something\", version = \"%s\")\n"
+ "public class %s extends Component {\n" + "}\n";

// Class used to test the class finder
public static final Class<NpmPackage> NPM_PACKAGE_CLASS = NpmPackage.class;
public static final Class<Component> COMPONENT_CLASS = Component.class;

@Rule
public TemporaryFolder externalModules = new TemporaryFolder();

URL[] urls;
private ClassFinder.DefaultClassFinder defaultClassFinder;
private List<String> componentClassNames;

@Before
public void setUp() throws Exception {
Expand All @@ -67,27 +73,43 @@ public void setUp() throws Exception {
createTestModule("module-2", "com.vaadin.flow.test.first",
"ComponentX", "1.0.0"),
createTestModule("module-3", "com.vaadin.flow.test.middle",
"ComponentA", "2.0.0") };

ClassLoader classLoader = new URLClassLoader(urls,
Thread.currentThread().getContextClassLoader());
defaultClassFinder = new ClassFinder.DefaultClassFinder(Set.of(
classLoader.loadClass("com.vaadin.flow.test.last.ComponentN"),
classLoader.loadClass("com.vaadin.flow.test.first.ComponentX"),
classLoader
.loadClass("com.vaadin.flow.test.middle.ComponentA")));
"ComponentA", "2.0.0"),
NPM_PACKAGE_CLASS.getProtectionDomain().getCodeSource()
.getLocation(),
COMPONENT_CLASS.getProtectionDomain().getCodeSource()
.getLocation() };

try (URLClassLoader classLoader = new URLClassLoader(urls,
Thread.currentThread().getContextClassLoader())) {
List<Class<?>> componentClasses = List.of(
classLoader
.loadClass("com.vaadin.flow.test.first.ComponentX"),
classLoader
.loadClass("com.vaadin.flow.test.last.ComponentN"),
classLoader.loadClass(
"com.vaadin.flow.test.middle.ComponentA"));
componentClassNames = componentClasses.stream().map(Class::getName)
.collect(Collectors.toList());
defaultClassFinder = new ClassFinder.DefaultClassFinder(
Set.copyOf(componentClasses));
}
}

@Test
public void getSubTypesOf_orderIsDeterministic() {
List<String> a1 = toList(new ReflectionsClassFinder(urls)
.getSubTypesOf(Component.class));
List<String> a2 = toList(
new ReflectionsClassFinder(urls[2], urls[0], urls[1])
.getSubTypesOf(Component.class));
List<String> a3 = toList(
new ReflectionsClassFinder(urls[1], urls[2], urls[0])
.getSubTypesOf(Component.class));
List<String> a1 = componentClassNames;
// Results of `getSubTypesOf` must be filtered because they also include
// all the subclasses from the JAR which contains `Component`.
List<String> a2 = new ReflectionsClassFinder(urls[2], urls[0], urls[1],
urls[4]).getSubTypesOf(COMPONENT_CLASS).stream()
.map(Class::getName)
.filter(name -> name.startsWith("com.vaadin.flow.test."))
.collect(Collectors.toList());
List<String> a3 = new ReflectionsClassFinder(urls[1], urls[4], urls[2],
urls[0]).getSubTypesOf(COMPONENT_CLASS).stream()
.map(Class::getName)
.filter(name -> name.startsWith("com.vaadin.flow.test."))
.collect(Collectors.toList());

Assert.assertEquals(a1, a2);
Assert.assertEquals(a2, a3);
Expand All @@ -96,13 +118,13 @@ public void getSubTypesOf_orderIsDeterministic() {
@Test
public void getAnnotatedClasses_orderIsDeterministic() {
List<String> a1 = toList(new ReflectionsClassFinder(urls)
.getAnnotatedClasses(NpmPackage.class));
.getAnnotatedClasses(NPM_PACKAGE_CLASS));
List<String> a2 = toList(
new ReflectionsClassFinder(urls[2], urls[0], urls[1])
.getAnnotatedClasses(NpmPackage.class));
new ReflectionsClassFinder(urls[3], urls[2], urls[0], urls[1])
.getAnnotatedClasses(NPM_PACKAGE_CLASS));
List<String> a3 = toList(
new ReflectionsClassFinder(urls[1], urls[2], urls[0])
.getAnnotatedClasses(NpmPackage.class));
new ReflectionsClassFinder(urls[1], urls[2], urls[3], urls[0])
.getAnnotatedClasses(NPM_PACKAGE_CLASS));

Assert.assertEquals(a1, a2);
Assert.assertEquals(a2, a3);
Expand All @@ -111,18 +133,17 @@ public void getAnnotatedClasses_orderIsDeterministic() {
@Test
public void getSubTypesOf_order_sameAsDefaultClassFinder() {
Assert.assertEquals(
toList(defaultClassFinder.getSubTypesOf(Component.class)),
toList(new ReflectionsClassFinder(urls)
.getSubTypesOf(Component.class)));
toList(defaultClassFinder.getSubTypesOf(COMPONENT_CLASS)),
componentClassNames);
}

@Test
public void getAnnotatedClasses_order_sameAsDefaultClassFinder() {
Assert.assertEquals(
toList(defaultClassFinder
.getAnnotatedClasses(NpmPackage.class)),
.getAnnotatedClasses(NPM_PACKAGE_CLASS)),
toList(new ReflectionsClassFinder(urls)
.getAnnotatedClasses(NpmPackage.class)));
.getAnnotatedClasses(NPM_PACKAGE_CLASS)));
}

@Test
Expand Down
Loading