Skip to content

Commit

Permalink
feat: add endpoint detection (#2938)
Browse files Browse the repository at this point in the history
* feat: add endpoint detection

* internal marker and test

* fix assert messages

* fix exising tests and add a new test for endpoint usage detector

* feat(engine): skip running Node and cleanup generated endpoints on empty OpenAPI

* chore: remove EndpointUsageDetector service file

* refactor: replace @InternalBrowserCallable with a hard-coded list

* test: update no-endponts IT module

* test(no-endpoints): assert that internal endpoints are not generated

* chore: formatting

* chore(endpoint): remove extra line

* fix(engine-core): do not fail if generated-file-list.txt does not exist

---------

Co-authored-by: Mikhail Shabarov <[email protected]>
Co-authored-by: Mikhail Shabarov <[email protected]>
Co-authored-by: Anton Platonov <[email protected]>
  • Loading branch information
4 people authored Feb 3, 2025
1 parent c2d0e1c commit bf63b54
Show file tree
Hide file tree
Showing 18 changed files with 445 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
*/
package com.vaadin.hilla;

import com.vaadin.hilla.signals.handler.SignalsHandler;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.Set;
import java.util.TreeMap;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -84,7 +86,8 @@ public class EndpointController {
*/
public static final String ENDPOINT_MAPPER_FACTORY_BEAN_QUALIFIER = "endpointMapperFactory";

private static final String SIGNALS_HANDLER_BEAN_NAME = "signalsHandler";
private static final Set<Class<?>> INTERNAL_BROWSER_CALLABLES = Set
.of(SignalsHandler.class);

private final ApplicationContext context;

Expand Down Expand Up @@ -135,24 +138,24 @@ public void registerEndpoints() {
endpointBeans.putAll(context.getBeansWithAnnotation(Endpoint.class));
endpointBeans
.putAll(context.getBeansWithAnnotation(BrowserCallable.class));
if (!endpointBeans.isEmpty()) {
HillaStats.reportHasEndpoint();
}

INTERNAL_BROWSER_CALLABLES.stream().map(context::getBeansOfType)
.forEach(endpointBeans::putAll);
var currentEndpointNames = endpointBeans.values().stream()
.map(endpointRegistry::registerEndpoint)
.collect(Collectors.toSet());
// remove obsolete endpoints
endpointRegistry.getEndpoints().keySet()
.retainAll(currentEndpointNames);

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

// Temporary Hack
VaadinService vaadinService = VaadinService.getCurrent();
if (vaadinService != null) {
this.vaadinService = vaadinService;
}

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@

import com.fasterxml.jackson.databind.node.ObjectNode;
import com.vaadin.flow.server.auth.AnonymousAllowed;
import com.vaadin.hilla.BrowserCallable;
import com.vaadin.hilla.EndpointInvocationException;
import com.vaadin.hilla.signals.core.event.ListStateEvent;
import com.vaadin.hilla.signals.core.registry.SecureSignalsRegistry;
import jakarta.annotation.Nullable;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Flux;

/**
* Handler Endpoint for Fullstack Signals' subscription and update events.
*/
@AnonymousAllowed
@BrowserCallable
@Component
public class SignalsHandler {

private static final String FEATURE_FLAG_ERROR_MESSAGE = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import java.util.List;
import java.util.Objects;

import com.vaadin.hilla.parser.core.OpenAPIFileType;
import io.swagger.v3.oas.models.OpenAPI;
import org.jspecify.annotations.NonNull;

import com.vaadin.hilla.engine.commandrunner.CommandNotFoundException;
Expand All @@ -16,6 +18,8 @@
import org.slf4j.LoggerFactory;

public final class GeneratorProcessor {
public static String GENERATED_FILE_LIST_NAME = "generated-file-list.txt";

private static final Logger logger = LoggerFactory
.getLogger(GeneratorProcessor.class);

Expand All @@ -36,6 +40,11 @@ public GeneratorProcessor(EngineConfiguration conf) {
}

public void process() throws GeneratorException {
if (isOpenAPIEmpty()) {
cleanup();
return;
}

var arguments = new ArrayList<Object>();
arguments.add(TSGEN_PATH);
prepareOutputDir(arguments);
Expand All @@ -62,6 +71,53 @@ public void process() throws GeneratorException {
}
}

private void cleanup() throws GeneratorException {
var generatedFilesListFile = outputDirectory
.resolve(GENERATED_FILE_LIST_NAME);
if (!generatedFilesListFile.toFile().exists()) {
logger.debug(
"Generated file list file does not exist, skipping cleanup.");
return;
}

logger.debug("Cleaning up old output.");
var generatedFilesList = List.<String> of();
try {
generatedFilesList = Files.readAllLines(generatedFilesListFile);
} catch (IOException e) {
throw new GeneratorException(
"Unable to read generated file list file", e);
}

try {
for (var line : generatedFilesList) {
var path = outputDirectory.resolve(line);
logger.debug("Removing generated file: {}", path);
Files.deleteIfExists(path);
// Also remove any empty parent directories
var dir = path.getParent();
while (dir.startsWith(outputDirectory)
&& !dir.equals(outputDirectory)
&& Files.isDirectory(dir) && Objects.requireNonNull(
dir.toFile().list()).length == 0) {
logger.debug("Removing unused generated directory: {}",
dir);
Files.deleteIfExists(dir);
}
}
} catch (IOException e) {
throw new GeneratorException("Unable to cleanup generated files",
e);
}

try {
Files.deleteIfExists(generatedFilesListFile);
} catch (IOException e) {
throw new GeneratorException(
"Unable to remove the generated file list file", e);
}
}

// Used to catch a checked exception in a lambda and handle it after
private static class LambdaException extends RuntimeException {
public LambdaException(Throwable cause) {
Expand Down Expand Up @@ -104,4 +160,23 @@ private void prepareVerbose(List<Object> arguments) {
arguments.add("-v");
}
}

private OpenAPI getOpenAPI() throws IOException {
String source = Files.readString(openAPIFile);
var mapper = OpenAPIFileType.JSON.getMapper();
var reader = mapper.reader();
return reader.readValue(source, OpenAPI.class);
}

private boolean isOpenAPIEmpty() {
try {
var openApi = getOpenAPI();
return (openApi.getPaths() == null || openApi.getPaths().isEmpty())
&& (openApi.getComponents() == null
|| openApi.getComponents().getSchemas() == null
|| openApi.getComponents().getSchemas().isEmpty());
} catch (IOException e) {
throw new GeneratorException("Unable to read OpenAPI json file", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
package com.vaadin.hilla.internal;

import com.vaadin.flow.server.ExecutionFailedException;
import com.vaadin.hilla.ApplicationContextProvider;
import com.vaadin.hilla.engine.EngineConfiguration;
import com.vaadin.hilla.internal.fixtures.CustomEndpoint;
import com.vaadin.hilla.internal.fixtures.EndpointNoValue;
import com.vaadin.hilla.internal.fixtures.MyEndpoint;
import org.junit.jupiter.api.Test;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ContextConfiguration;

import static org.junit.jupiter.api.Assertions.assertThrowsExactly;

class AbstractTaskEndpointGeneratorTest extends TaskTest {
class AbstractTaskEndpointGeneratorTest extends EndpointsTaskTest {
@Test
void shouldThrowIfEngineConfigurationIsNull() {
assertThrowsExactly(NullPointerException.class, () -> {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package com.vaadin.hilla.internal;

import com.vaadin.flow.server.frontend.FrontendUtils;
import com.vaadin.hilla.ApplicationContextProvider;
import com.vaadin.hilla.internal.fixtures.CustomEndpoint;
import com.vaadin.hilla.internal.fixtures.EndpointNoValue;
import com.vaadin.hilla.internal.fixtures.MyEndpoint;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.util.FileSystemUtils;

import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Stream;

@SpringBootTest(classes = { CustomEndpoint.class, EndpointNoValue.class,
MyEndpoint.class, ApplicationContextProvider.class })
public class EndpointsTaskTest extends TaskTest {

static private Path npmDependenciesTempDirectory;

@BeforeAll
public static void setupNpmDependencies()
throws IOException, FrontendUtils.CommandExecutionException,
InterruptedException, URISyntaxException {
npmDependenciesTempDirectory = Files
.createTempDirectory(EndpointsTaskTest.class.getName());

Path packagesPath = Path
.of(Objects.requireNonNull(EndpointsTaskTest.class
.getClassLoader().getResource("")).toURI())
.getParent() // target
.getParent() // engine-runtime
.getParent() // java
.getParent(); // packages

Path projectRoot = packagesPath.getParent();
Files.copy(projectRoot.resolve(".npmrc"),
npmDependenciesTempDirectory.resolve(".npmrc"));
var tsPackagesDirectory = packagesPath.resolve("ts");

var shellCmd = FrontendUtils.isWindows() ? Stream.of("cmd.exe", "/c")
: Stream.<String> empty();

var npmCmd = Stream.of("npm", "--no-update-notifier", "--no-audit",
"install", "--no-save", "--install-links");

var generatorFiles = Files.list(tsPackagesDirectory)
.map(Path::toString);

var command = Stream.of(shellCmd, npmCmd, generatorFiles)
.flatMap(Function.identity()).toList();

var processBuilder = FrontendUtils.createProcessBuilder(command)
.directory(npmDependenciesTempDirectory.toFile())
.redirectOutput(ProcessBuilder.Redirect.INHERIT)
.redirectError(ProcessBuilder.Redirect.INHERIT);
var exitCode = processBuilder.start().waitFor();
if (exitCode != 0) {
throw new FrontendUtils.CommandExecutionException(exitCode);
}
}

@BeforeEach
public void copyNpmDependencies() throws IOException {
FileSystemUtils.copyRecursively(npmDependenciesTempDirectory,
getTemporaryDirectory());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package com.vaadin.hilla.internal;

import com.vaadin.flow.server.ExecutionFailedException;
import com.vaadin.flow.server.frontend.TaskGenerateEndpoint;
import com.vaadin.flow.server.frontend.TaskGenerateOpenAPI;
import com.vaadin.hilla.ApplicationContextProvider;
import com.vaadin.hilla.engine.GeneratorProcessor;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.springframework.beans.BeansException;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.ApplicationContext;

import javax.annotation.Nonnull;
import java.io.IOException;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Objects;
import java.util.function.Consumer;

import static org.junit.jupiter.api.Assertions.*;

@SpringBootTest(classes = {
NoEndpointsTaskTest.NoopApplicationContextProvider.class })
public class NoEndpointsTaskTest extends TaskTest {
private TaskGenerateOpenAPI taskGenerateOpenApi;
private TaskGenerateEndpoint taskGenerateEndpoint;

@Autowired
ApplicationContext applicationContext;

@Test
public void should_GenerateEmptySchema_when_NoEndpointsFound()
throws ExecutionFailedException, IOException, URISyntaxException {
// Mock ApplicationContextProvider static API to prevent interference
// with other tests.
try (var mockApplicationContextProvider = Mockito
.mockStatic(ApplicationContextProvider.class)) {
mockApplicationContextProvider
.when(ApplicationContextProvider::getApplicationContext)
.thenReturn(applicationContext);
mockApplicationContextProvider
.when(() -> ApplicationContextProvider
.runOnContext(Mockito.any()))
.thenAnswer(invocationOnMock -> {
invocationOnMock
.<Consumer<ApplicationContext>> getArgument(0)
.accept(applicationContext);
return null;
});

// Create files resembling output for previously existing endpoints
var outputDirectory = Files.createDirectory(
getTemporaryDirectory().resolve(getOutputDirectory()));
var generatedFileListPath = outputDirectory
.resolve(GeneratorProcessor.GENERATED_FILE_LIST_NAME);
var referenceFileListPath = Path.of(Objects
.requireNonNull(getClass().getResource(
GeneratorProcessor.GENERATED_FILE_LIST_NAME))
.toURI());
Files.copy(referenceFileListPath, generatedFileListPath);
var referenceFileList = Files.readAllLines(referenceFileListPath);
for (String line : referenceFileList) {
var path = outputDirectory.resolve(line);
Files.createDirectories(path.getParent());
Files.createFile(path);
}
var arbitraryGeneratedFile = outputDirectory.resolve("vaadin.ts");
Files.createFile(arbitraryGeneratedFile);

taskGenerateOpenApi = new TaskGenerateOpenAPIImpl(
getEngineConfiguration());
taskGenerateEndpoint = new TaskGenerateEndpointImpl(
getEngineConfiguration());

taskGenerateOpenApi.execute();

var generatedOpenAPI = getGeneratedOpenAPI();

assertNull(generatedOpenAPI.getTags(),
"Expected OpenAPI tags to be null");
assertTrue(generatedOpenAPI.getPaths().isEmpty(),
"Expected OpenAPI paths to be empty");
assertNull(generatedOpenAPI.getComponents(),
"Expected OpenAPI schemas to be null");

assertDoesNotThrow(taskGenerateEndpoint::execute,
"Expected to not fail without npm dependencies");

assertFalse(generatedFileListPath.toFile().exists(),
"Expected file list to be deleted");
for (String line : referenceFileList) {
var path = outputDirectory.resolve(line);
assertFalse(path.toFile().exists(),
String.format("Expected file %s to be deleted", path));
}
assertTrue(arbitraryGeneratedFile.toFile().exists(),
"Expected non-Hilla generated file to not be deleted");
}
}

static class NoopApplicationContextProvider
extends ApplicationContextProvider {
@Override
public void setApplicationContext(
@Nonnull ApplicationContext applicationContext)
throws BeansException {
// do nothing
}
}
}
Loading

0 comments on commit bf63b54

Please sign in to comment.