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: improved endpoint detection #21009

Merged
merged 13 commits into from
Feb 25, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

import java.io.File;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.Set;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -47,6 +49,16 @@ public TaskGenerateOpenAPI createTaskGenerateOpenAPI(Options options) {
return new TestTaskGenerateOpenAPI(options);
}

@Override
public Set<Class<? extends Annotation>> getBrowserCallableAnnotations() {
return Set.of();
}

@Override
public boolean hasBrowserCallables(Options options) {
return true;
}

/**
* An abstract parent for the test endpoints generator tasks.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ public void should_useHillaEngine_withNodeUpdater()
EndpointRequestUtil.class, Mockito.CALLS_REAL_METHODS)) {
util.when(() -> EndpointRequestUtil.isHillaAvailable(Mockito.any()))
.thenReturn(true);
util.when(() -> EndpointRequestUtil
.areHillaEndpointsUsed(Mockito.any())).thenReturn(true);
BuildFrontendUtil.runNodeUpdater(adapter);
}

Expand All @@ -168,10 +170,10 @@ public void should_useHillaEngine_withNodeUpdater()
// Hilla Engine requires npm install, the order of execution is critical
final TaskRunNpmInstall taskRunNpmInstall = construction.constructed()
.get(0);
InOrder inOrder = Mockito.inOrder(taskRunNpmInstall,
taskGenerateOpenAPI, taskGenerateEndpoint);
inOrder.verify(taskRunNpmInstall).execute();
InOrder inOrder = Mockito.inOrder(taskGenerateOpenAPI,
taskRunNpmInstall, taskGenerateEndpoint);
inOrder.verify(taskGenerateOpenAPI).execute();
inOrder.verify(taskRunNpmInstall).execute();
inOrder.verify(taskGenerateEndpoint).execute();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import jakarta.servlet.http.HttpServletRequest;
import java.io.Serializable;

import com.vaadin.flow.server.frontend.EndpointGeneratorTaskFactory;
import com.vaadin.flow.server.frontend.Options;
import com.vaadin.flow.server.frontend.scanner.ClassFinder;

/**
Expand Down Expand Up @@ -86,4 +88,22 @@ static boolean isHillaAvailable(ClassFinder classFinder) {
return false;
}
}

/**
* Checks if Hilla is available and Hilla endpoints are used in the project.
*
* @return {@code true} if Hilla is available and Hilla views are used,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @return {@code true} if Hilla is available and Hilla views are used,
* @return {@code true} if Hilla is available and Hilla endpoints are used,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

* {@code false} otherwise
*/
static boolean areHillaEndpointsUsed(Options options) {
if (!EndpointRequestUtil.isHillaAvailable()) {
return false;
}
EndpointGeneratorTaskFactory endpointGeneratorTaskFactory = options
.getLookup().lookup(EndpointGeneratorTaskFactory.class);
if (endpointGeneratorTaskFactory != null) {
return endpointGeneratorTaskFactory.hasBrowserCallables(options);
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.vaadin.flow.internal.JacksonUtils;
import com.vaadin.flow.internal.StringUtil;
import com.vaadin.flow.internal.UsageStatistics;
import com.vaadin.flow.internal.hilla.EndpointRequestUtil;
import com.vaadin.flow.server.Constants;
import com.vaadin.flow.server.LoadDependenciesOnStartup;
import com.vaadin.flow.server.Mode;
Expand Down Expand Up @@ -68,9 +69,11 @@ public static boolean needsBuild(Options options,
try {
boolean needsBuild;
if (mode.isProduction()) {
if (options.isForceProductionBuild() || FrontendUtils
.isHillaUsed(options.getFrontendDirectory(),
options.getClassFinder())) {
if (options.isForceProductionBuild()
|| FrontendUtils.isHillaUsed(
options.getFrontendDirectory(),
options.getClassFinder())
|| EndpointRequestUtil.areHillaEndpointsUsed(options)) {
if (options.isForceProductionBuild()) {
UsageStatistics.markAsUsed("flow/prod-build-requested",
null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@

package com.vaadin.flow.server.frontend;

import java.lang.annotation.Annotation;
import java.util.Set;

/**
* A factory for creating Vaadin Endpoint generator tasks.
* <p>
Expand Down Expand Up @@ -43,4 +46,21 @@ public interface EndpointGeneratorTaskFactory {
* @return an endpoint task that generates open api json file.
*/
TaskGenerateOpenAPI createTaskGenerateOpenAPI(Options options);

/**
* Fetches all endpoint-type annotations from Hilla configuration
*
* @return Set of endpoint-type annotations
*/
Set<Class<? extends Annotation>> getBrowserCallableAnnotations();

/**
* Determines the presence of annotations (e.g. BrowserCallable or Endpoint)
* which require Flow to add Hilla packages to the build.
*
* @param options
* the task options
* @return {@code true} if annotations are present, {@code false} otherwise
*/
boolean hasBrowserCallables(Options options);
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.util.stream.Stream;

import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import jakarta.servlet.ServletContext;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ public class NodeTasks implements FallibleCommand {
TaskGenerateWebComponentBootstrap.class,
TaskGenerateFeatureFlags.class,
TaskInstallFrontendBuildPlugins.class,
TaskGenerateOpenAPI.class,
TaskUpdatePackages.class,
TaskRunNpmInstall.class,
TaskGenerateOpenAPI.class,
TaskGenerateEndpoint.class,
TaskCopyFrontendFiles.class,
TaskCopyLocalFrontendFiles.class,
Expand Down Expand Up @@ -319,6 +319,9 @@ private void addEndpointServicesTasks(Options options) {
if (!EndpointRequestUtil.isHillaAvailable(options.getClassFinder())) {
return;
}
if (!EndpointRequestUtil.areHillaEndpointsUsed(options)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if views are there but no endpoints yet?
I'd do it in this way:

  1. If Hilla not available - return
  2. if endpoints are used or views are used - continue, else - return.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the logic

return;
}
Lookup lookup = options.getLookup();
EndpointGeneratorTaskFactory endpointGeneratorTaskFactory = lookup
.lookup(EndpointGeneratorTaskFactory.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.vaadin.experimental.FeatureFlags;
import com.vaadin.flow.internal.JacksonUtils;
import com.vaadin.flow.internal.JsonDecodingException;
import com.vaadin.flow.internal.hilla.EndpointRequestUtil;
import com.vaadin.flow.server.Constants;
import com.vaadin.flow.server.frontend.scanner.ClassFinder;
import com.vaadin.flow.server.frontend.scanner.FrontendDependencies;
Expand Down Expand Up @@ -620,7 +621,8 @@ private ObjectNode generateVersionsFromPackageJson(JsonNode packageJson) {
private void putHillaComponentsDependencies(
Map<String, String> dependencies, String packageJsonKey) {
if (FrontendUtils.isHillaUsed(options.getFrontendDirectory(),
options.getClassFinder())) {
options.getClassFinder())
|| EndpointRequestUtil.areHillaEndpointsUsed(options)) {
if (options.isReactEnabled()) {
dependencies.putAll(readDependenciesIfAvailable(
"hilla/components/react", packageJsonKey));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ public void should_useHillaEngine_whenEnabled()
EndpointRequestUtil.class, Mockito.CALLS_REAL_METHODS)) {
util.when(() -> EndpointRequestUtil.isHillaAvailable(Mockito.any()))
.thenReturn(true);

util.when(() -> EndpointRequestUtil
.areHillaEndpointsUsed(Mockito.any())).thenReturn(true);
new NodeTasks(options).execute();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ public void should_generateOpenApi() throws Exception {
EndpointRequestUtil.class, Mockito.CALLS_REAL_METHODS)) {
util.when(() -> EndpointRequestUtil.isHillaAvailable(Mockito.any()))
.thenReturn(true);
util.when(() -> EndpointRequestUtil
.areHillaEndpointsUsed(Mockito.any())).thenReturn(true);
devModeStartupListener.onStartup(classes, servletContext);
handler = getDevModeHandler();
waitForDevServer();
Expand Down Expand Up @@ -144,6 +146,8 @@ public void should_generateTs_files() throws Exception {
EndpointRequestUtil.class, Mockito.CALLS_REAL_METHODS)) {
util.when(() -> EndpointRequestUtil.isHillaAvailable(Mockito.any()))
.thenReturn(true);
util.when(() -> EndpointRequestUtil
.areHillaEndpointsUsed(Mockito.any())).thenReturn(true);
devModeStartupListener.onStartup(classes, servletContext);
handler = getDevModeHandler();
waitForDevServer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@

import java.io.File;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.StandardOpenOption;
import java.util.Set;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -47,6 +49,16 @@ public TaskGenerateOpenAPI createTaskGenerateOpenAPI(Options options) {
return new TestTaskGenerateOpenAPI(options);
}

@Override
public Set<Class<? extends Annotation>> getBrowserCallableAnnotations() {
return Set.of();
}

@Override
public boolean hasBrowserCallables(Options options) {
return true;
}

/**
* An abstract parent for the test endpoints generator tasks.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,8 @@ public void should_generateOpenApi_when_EndpointPresents()
EndpointRequestUtil.class, Mockito.CALLS_REAL_METHODS)) {
util.when(() -> EndpointRequestUtil
.isHillaAvailable(Mockito.any())).thenReturn(true);
util.when(() -> EndpointRequestUtil
.areHillaEndpointsUsed(Mockito.any())).thenReturn(true);
devModeStartupListener.onStartup(classes, servletContext);
handler = getDevModeHandler();
waitForDevServer();
Expand Down Expand Up @@ -425,6 +427,8 @@ public void should_generateTs_files() throws Exception {
EndpointRequestUtil.class, Mockito.CALLS_REAL_METHODS)) {
util.when(() -> EndpointRequestUtil
.isHillaAvailable(Mockito.any())).thenReturn(true);
util.when(() -> EndpointRequestUtil
.areHillaEndpointsUsed(Mockito.any())).thenReturn(true);
devModeStartupListener.onStartup(classes, servletContext);
handler = getDevModeHandler();
waitForDevServer();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import jakarta.servlet.ServletContextListener;
import jakarta.servlet.ServletException;
import jakarta.servlet.annotation.HandlesTypes;

import java.io.File;
import java.io.IOException;
import java.io.Serializable;
Expand Down Expand Up @@ -65,6 +64,7 @@
import com.vaadin.flow.di.LookupInitializer;
import com.vaadin.flow.internal.DevModeHandlerManager;
import com.vaadin.flow.router.HasErrorParameter;
import com.vaadin.flow.router.Layout;
import com.vaadin.flow.router.Route;
import com.vaadin.flow.router.RouteAlias;
import com.vaadin.flow.router.RouteConfiguration;
Expand All @@ -75,7 +75,7 @@
import com.vaadin.flow.server.RouteRegistry;
import com.vaadin.flow.server.VaadinServletContext;
import com.vaadin.flow.server.communication.IndexHtmlRequestHandler;
import com.vaadin.flow.router.Layout;
import com.vaadin.flow.server.frontend.EndpointGeneratorTaskFactory;
import com.vaadin.flow.server.startup.AbstractRouteRegistryInitializer;
import com.vaadin.flow.server.startup.AnnotationValidator;
import com.vaadin.flow.server.startup.ApplicationConfiguration;
Expand Down Expand Up @@ -559,6 +559,14 @@ public void failFastContextInitialized(ServletContextEvent event)
collectHandleTypes(devModeHandlerManager.getHandlesTypes(),
annotations, superTypes);

EndpointGeneratorTaskFactory endpointGeneratorTaskFactory = lookup
.lookup(EndpointGeneratorTaskFactory.class);

if (endpointGeneratorTaskFactory != null) {
annotations.addAll(endpointGeneratorTaskFactory
.getBrowserCallableAnnotations());
}
Comment on lines +565 to +568
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be covered by tests, so needs a unit test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added test


Set<Class<?>> classes = findClassesForDevMode(basePackages,
annotations, superTypes);

Expand Down
Loading