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

feat: add endpoint detection #2938

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package com.vaadin.hilla;

import com.vaadin.flow.server.frontend.EndpointUsageDetector;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

/**
* Marker interface to be used by {@link EndpointUsageDetector} for detecting
* framework's internal {@link BrowserCallable} and {@link Endpoint} endpoints
* that shouldn't start the endpoint generator per se.
* <p>
* For internal use only. May be renamed or removed in a future release.
*
* @author Vaadin Ltd
* @since 24.7
*/
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface InternalBrowserCallable {
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.vaadin.flow.server.auth.AnonymousAllowed;
import com.vaadin.hilla.BrowserCallable;
import com.vaadin.hilla.EndpointInvocationException;
import com.vaadin.hilla.InternalBrowserCallable;
import com.vaadin.hilla.signals.core.event.ListStateEvent;
import com.vaadin.hilla.signals.core.registry.SecureSignalsRegistry;
import jakarta.annotation.Nullable;
Expand All @@ -30,6 +31,7 @@
*/
@AnonymousAllowed
@BrowserCallable
@InternalBrowserCallable
public class SignalsHandler {

private static final String FEATURE_FLAG_ERROR_MESSAGE = """
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.vaadin.hilla.startup;

import java.util.stream.Stream;

import com.vaadin.flow.server.frontend.EndpointUsageDetector;
import com.vaadin.flow.server.frontend.Options;
import com.vaadin.flow.server.frontend.scanner.ClassFinder;
import com.vaadin.hilla.BrowserCallable;
import com.vaadin.hilla.Endpoint;
import com.vaadin.hilla.InternalBrowserCallable;

/**
* Implementation of EndpointUsageDetector which determines if the endpoint
* generator task needs to be run.
*/
public class EndpointUsageDetectorImpl implements EndpointUsageDetector {

@Override
public boolean areEndpointsUsed(Options options) {
ClassFinder classFinder = options.getClassFinder();

return Stream.concat(
classFinder.getAnnotatedClasses(Endpoint.class).stream(),
classFinder.getAnnotatedClasses(BrowserCallable.class).stream())
.anyMatch(annotatedClass -> !annotatedClass
.isAnnotationPresent(InternalBrowserCallable.class));
Comment on lines +22 to +26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Searching in the classpath for annotated classes does not necessarily match the reality of the endpoints in a Hilla application.

Until 24.6, the list of packages where to search for endpoints is configurable and, if not configured, defaults to search inside target/classes for backwards compatibility.

Starting from 24.7, endpoints must be Spring beans (which was the case anyway 99% of the times).

So, in both cases the presence of an annotated class does not necessarily mean that an endpoint will be compiled to TypeScript and used in communication with clients.

We have a registry of endpoints, but of course that comes probably too late for your needs.

Also, we should define a common way to skip internal endpoints, to use also in

endpointBeans.keySet().stream()
.filter(name -> !name.equals(SIGNALS_HANDLER_BEAN_NAME))
.findAny().ifPresent(name -> HillaStats.reportHasEndpoint());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, this is where we define the list of endpoints using Spring ApplicationContext:

public static List<Class<?>> findBrowserCallables(
EngineConfiguration engineConfiguration,
ApplicationContext applicationContext) {
return engineConfiguration.getEndpointAnnotations().stream()
.map(applicationContext::getBeansWithAnnotation)
.map(Map::values).flatMap(Collection::stream)
// maps to original class when proxies are found
// (also converts to class in all cases)
.map(AopProxyUtils::ultimateTargetClass).distinct()
.collect(Collectors.toList());
}

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.vaadin.hilla.startup.EndpointUsageDetectorImpl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.server.frontend.EndpointGeneratorTaskFactory;
import com.vaadin.flow.server.frontend.EndpointUsageDetector;
import com.vaadin.flow.server.frontend.NodeTasks;
import com.vaadin.flow.server.frontend.Options;
import com.vaadin.flow.server.frontend.scanner.ClassFinder;
Expand All @@ -34,6 +35,12 @@ public static class ConnectEndpointsForTesting {
public void setUp()
throws IOException, NoSuchFieldException, IllegalAccessException {
Lookup mockLookup = Mockito.mock(Lookup.class);
EndpointUsageDetector endpointUsageDetector = Mockito
.mock(EndpointUsageDetector.class);
Mockito.when(endpointUsageDetector
.areEndpointsUsed(Mockito.any(Options.class))).thenReturn(true);
Mockito.doReturn(endpointUsageDetector).when(mockLookup)
.lookup(EndpointUsageDetector.class);
Mockito.doReturn(new EndpointGeneratorTaskFactoryImpl())
.when(mockLookup).lookup(EndpointGeneratorTaskFactory.class);
Mockito.doReturn(new DefaultClassFinder(Set.of(
Expand Down
11 changes: 11 additions & 0 deletions packages/java/hilla/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,17 @@
<artifactId>hilla-engine-runtime</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.reflections</groupId>
<artifactId>reflections</artifactId>
<version>0.10.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.vaadin.hilla;

import java.util.Set;

import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.reflections.Reflections;
import org.reflections.scanners.Scanners;

/**
* Validates that all endpoint classes in sources are annotated with
* {@link InternalBrowserCallable} to not trigger endpoint generator if
* endpoints are not actually used.
*/
public class InternalBrowserCallableTest {

private static final String BASE_PACKAGE = "com.vaadin";

@Test
public void testBrowserCallableAndEndpointAnnotationsMarkedAsInternal() {
Reflections reflections = new Reflections(BASE_PACKAGE,
Scanners.TypesAnnotated);
Set<Class<?>> annotatedClasses = reflections
.getTypesAnnotatedWith(BrowserCallable.class);
Set<Class<?>> endpointClasses = reflections
.getTypesAnnotatedWith(Endpoint.class);
annotatedClasses.addAll(endpointClasses);

for (Class<?> clazz : annotatedClasses) {
if (!clazz.getName().contains(".test.") && !clazz
.isAnnotationPresent(InternalBrowserCallable.class)) {
Assertions.fail("Class " + clazz.getName()
+ " is annotated with @BrowserCallable or @Endpoint, but missing @InternalBrowserCallable.");
}
}
}

@Test
public void testInternalBrowserCallableHasBrowserCallableOrEndpoint()
throws Exception {
Reflections reflections = new Reflections(BASE_PACKAGE,
Scanners.TypesAnnotated);
Set<Class<?>> annotatedClasses = reflections
.getTypesAnnotatedWith(InternalBrowserCallable.class);

for (Class<?> clazz : annotatedClasses) {
if (!clazz.isAnnotationPresent(BrowserCallable.class)
&& !clazz.isAnnotationPresent(Endpoint.class)) {
Assertions.fail("Class " + clazz.getName()
+ " is annotated with @InternalBrowserCallable and must be annotated with @BrowserCallable or @Endpoint.");
}
}
}
}
64 changes: 64 additions & 0 deletions packages/java/tests/spring/no-endpoints/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?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>
<parent>
<groupId>com.vaadin</groupId>
<artifactId>tests-spring</artifactId>
<version>24.6-SNAPSHOT</version>
</parent>
<artifactId>tests-spring-no-endpoints</artifactId>
<name>ITs for No Endpoints cases</name>
<packaging>jar</packaging>

<properties>
<maven.deploy.skip>true</maven.deploy.skip>
<formatter.basedir>${project.parent.parent.parent.basedir}</formatter.basedir>
</properties>

<dependencies>
<dependency>
<groupId>com.vaadin</groupId>
<artifactId>hilla</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>com.vaadin</groupId>
<artifactId>hilla-maven-plugin</artifactId>
<configuration>
<parser>
<packages>
<package>com.example.application</package>
</packages>
</parser>
</configuration>
<executions>
<execution>
<goals>
<!-- The test here assumes the build creates frontend files -->
<goal>build-frontend</goal>
</goals>
</execution>
</executions>
</plugin>

<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<configuration>
<includeSystemScope>true</includeSystemScope>
</configuration>
</plugin>
</plugins>
</build>

</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.example.application;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

/**
* The entry point of the Spring Boot application.
*/
@SpringBootApplication
public class Application {

public static void main(String[] args) {
SpringApplication.run(Application.class, args);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.example.application;

import com.vaadin.hilla.Endpoint;
import com.vaadin.hilla.InternalBrowserCallable;

@Endpoint
@InternalBrowserCallable
public class InternalBrowserCallableExample {
public String exampleMethod() {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.example.application;

import com.vaadin.hilla.Endpoint;
import com.vaadin.hilla.InternalBrowserCallable;

@Endpoint
@InternalBrowserCallable
public class InternalEndpointExample {
public String exampleMethod() {
return null;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
server.port=8888
vaadin.devmode.liveReload.enabled=false
logging.level.org.atmosphere = warn
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.example.application;

import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.junit.Test;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class InternalEndpointIT {
Path frontendDir = Paths.get("frontend", "generated");

@Test
public void shouldNotRunEndpointGeneratorForOnlyInternalEndpointsPresent() {
assertFalse(Files.exists(frontendDir.resolve("InternalBrowserCallableExample.ts")));
assertFalse(Files.exists(frontendDir.resolve("InternalEndpointExample.ts")));
assertFalse(Files.exists(frontendDir
.resolve("com/example/application/InternalBrowserCallableExample.ts")));
assertFalse(Files.exists(frontendDir
.resolve("com/example/application/InternalEndpointExample.ts")));
}


}
Loading