Skip to content

Commit

Permalink
refactor: get Executor service from Lookup instead init params (#9572)
Browse files Browse the repository at this point in the history
fixes #9570
  • Loading branch information
Denis authored Dec 7, 2020
1 parent 16f7585 commit dedb8dd
Show file tree
Hide file tree
Showing 10 changed files with 465 additions and 138 deletions.
126 changes: 114 additions & 12 deletions flow-server/src/main/java/com/vaadin/flow/di/Lookup.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,43 @@
package com.vaadin.flow.di;

import java.util.Collection;
import java.util.Collections;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.server.VaadinContext;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinServlet;

/**
* Provides a way to discover services used by Flow (SPI). Dependency injection
* frameworks can provide an implementation that manages instances according to
* the conventions of that framework.
* Provides a way to discover services used by Flow (SPI).
* <p>
* This is similar to the {@link Instantiator} class but a {@link Lookup}
* instance is available even before a {@link VaadinService} instance is created
* (and as a consequence there is no yet an {@link Instantiator} instance).
* A lookup instance may be created based on a service, see
* {@link #of(Object, Class...)}. Several lookup instances may be combined via
* {@link #compose(Lookup, Lookup)} method which allows to make a lookup
* instance based on a number of services. The resulting lookup instance may be
* used in internal Flow code to transfer data in the unified way which allows
* to change the available data types during the code evolution without changing
* the internal API (like arguments in methods and constructors).
* <p>
* The {@link Lookup} instance and the {@link VaadinContext} has one to one
* mapping and is available even before a {@link DeploymentConfiguration} ( and
* {@link VaadinServlet}) is created. So this is kind of a singleton for a Web
* Application. As a consequence it provides and may return only web app
* singleton services.
* There is the "global" application {@link Lookup} instance and the
* {@link VaadinContext}. It has one to one mapping and is available even before
* a {@link DeploymentConfiguration} (and {@link VaadinServlet}) is created. So
* this is kind of a singleton for a Web Application. As a consequence it
* provides and may return only web app singleton services. Dependency injection
* frameworks can provide an implementation for the application {@code Lookup}
* which manages instances according to the conventions of that framework.
* <p>
* This is the code which one may use to get the {@link Lookup} instance:
* The application {@code Lookup} is similar to the {@link Instantiator} class
* but a {@link Lookup} instance is available even before a
* {@link VaadinService} instance is created (and as a consequence there is no
* yet an {@link Instantiator} instance).
* <p>
* This is the code which one may use to get the application {@link Lookup}
* instance:
*
* <pre>
* <code>
Expand Down Expand Up @@ -89,4 +104,91 @@ public interface Lookup {
* returned)
*/
<T> Collection<T> lookupAll(Class<T> serviceClass);

/**
* Creates a lookup which contains (only) the provided {@code service} as
* instance of given {@code serviceTypes}.
* <p>
* This method may be used to create a temporary lookup which then can be
* used to extend an existing lookup via {@link #compose(Lookup, Lookup)}.
*
* @param <T>
* the service type
* @param service
* the service object
* @param serviceTypes
* the supertypes of the service which may be used to access the
* service
* @return a lookup initialized with the given {@code service}
*/
@SafeVarargs
static <T> Lookup of(T service, Class<? super T>... serviceTypes) {
Objects.requireNonNull(service);
Set<Class<? super T>> services = Stream.of(serviceTypes).peek(type -> {
if (!type.isInstance(service)) {
throw new IllegalArgumentException(
"Service type" + service.getClass().getName()
+ " is not a subtype of " + type.getName());
}
}).collect(Collectors.toSet());
return new Lookup() {

@Override
public <U> Collection<U> lookupAll(Class<U> serviceClass) {
U service = lookup(serviceClass);
return service == null ? Collections.emptyList()
: Collections.singleton(service);
}

@Override
public <U> U lookup(Class<U> serviceClass) {
if (services.contains(serviceClass)) {
return serviceClass.cast(service);
}
return null;
}
};
}

/**
* Make a composite lookup which contains the services from both
* {@code lookup1} and {@code lookup2}.
* <p>
* {@link #lookup(Class)} method will return the service from the first
* lookup if it's not null and fallbacks to the {@code lookup2} otherwise.
* So the first lookup takes precedence. The method
* {@link #lookupAll(Class)} simply combines all the services from both
* lookups.
* <p>
* The resulting lookup is intended to be a "temporary" (short living)
* lookup to extend an existing lookup with some additional data which is
* required only in some isolated object.
*
* @param lookup1
* the first lookup to compose
* @param lookup2
* the second lookup to compose
* @return the composite lookup
*/
static Lookup compose(Lookup lookup1, Lookup lookup2) {
return new Lookup() {

@Override
public <T> Collection<T> lookupAll(Class<T> serviceClass) {
return Stream
.concat(lookup1.lookupAll(serviceClass).stream(),
lookup2.lookupAll(serviceClass).stream())
.collect(Collectors.toList());
}

@Override
public <T> T lookup(Class<T> serviceClass) {
T service = lookup1.lookup(serviceClass);
if (service == null) {
return lookup2.lookup(serviceClass);
}
return service;
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.internal.BrowserLiveReload;
import com.vaadin.flow.internal.Pair;
Expand Down Expand Up @@ -136,16 +137,18 @@ public final class DevModeHandler implements RequestHandler {

private final File npmFolder;

private DevModeHandler(DeploymentConfiguration config, int runningPort,
File npmFolder, CompletableFuture<Void> waitFor) {
private DevModeHandler(Lookup lookup, int runningPort, File npmFolder,
CompletableFuture<Void> waitFor) {

this.npmFolder = npmFolder;
port = runningPort;
DeploymentConfiguration config = lookup
.lookup(DeploymentConfiguration.class);
reuseDevServer = config.reuseDevServer();
devServerPortFile = getDevServerPortFile(npmFolder);

// Check whether executor is provided by the caller (framework)
Object service = config.getInitParameters().get(Executor.class);
Executor service = lookup.lookup(Executor.class);

BiConsumer<Void, ? super Throwable> action = (value, exception) -> {
// this will throw an exception if an exception has been thrown by
Expand All @@ -154,21 +157,19 @@ private DevModeHandler(DeploymentConfiguration config, int runningPort,
runOnFutureComplete(config);
};

if (service instanceof Executor) {
// if there is an executor use it to run the task
devServerStartFuture = waitFor.whenCompleteAsync(action,
(Executor) service);
} else {
if (service == null) {
devServerStartFuture = waitFor.whenCompleteAsync(action);
} else {
// if there is an executor use it to run the task
devServerStartFuture = waitFor.whenCompleteAsync(action, service);
}

}

/**
* Start the dev mode handler if none has been started yet.
*
* @param configuration
* deployment configuration
* @param lookup
* the provided lookup to get required data
* @param npmFolder
* folder with npm configuration files
* @param waitFor
Expand All @@ -177,18 +178,18 @@ private DevModeHandler(DeploymentConfiguration config, int runningPort,
*
* @return the instance in case everything is alright, null otherwise
*/
public static DevModeHandler start(DeploymentConfiguration configuration,
File npmFolder, CompletableFuture<Void> waitFor) {
return start(0, configuration, npmFolder, waitFor);
public static DevModeHandler start(Lookup lookup, File npmFolder,
CompletableFuture<Void> waitFor) {
return start(0, lookup, npmFolder, waitFor);
}

/**
* Start the dev mode handler if none has been started yet.
*
* @param runningPort
* port on which Webpack is listening.
* @param configuration
* deployment configuration
* @param lookup
* the provided lookup to get required data
* @param npmFolder
* folder with npm configuration files
* @param waitFor
Expand All @@ -197,16 +198,17 @@ public static DevModeHandler start(DeploymentConfiguration configuration,
*
* @return the instance in case everything is alright, null otherwise
*/
public static DevModeHandler start(int runningPort,
DeploymentConfiguration configuration, File npmFolder,
CompletableFuture<Void> waitFor) {
public static DevModeHandler start(int runningPort, Lookup lookup,
File npmFolder, CompletableFuture<Void> waitFor) {
DeploymentConfiguration configuration = lookup
.lookup(DeploymentConfiguration.class);
if (configuration.isProductionMode()
|| !configuration.enableDevServer()) {
return null;
}
if (atomicHandler.get() == null) {
atomicHandler.compareAndSet(null, createInstance(runningPort,
configuration, npmFolder, waitFor));
atomicHandler.compareAndSet(null,
createInstance(runningPort, lookup, npmFolder, waitFor));
}
return getDevModeHandler();
}
Expand Down Expand Up @@ -269,11 +271,9 @@ private RuntimeException getCause(Throwable exception) {
}
}

private static DevModeHandler createInstance(int runningPort,
DeploymentConfiguration configuration, File npmFolder,
CompletableFuture<Void> waitFor) {
return new DevModeHandler(configuration, runningPort, npmFolder,
waitFor);
private static DevModeHandler createInstance(int runningPort, Lookup lookup,
File npmFolder, CompletableFuture<Void> waitFor) {
return new DevModeHandler(lookup, runningPort, npmFolder, waitFor);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import com.vaadin.flow.component.dependency.JsModule;
import com.vaadin.flow.component.dependency.NpmPackage;
import com.vaadin.flow.component.page.AppShellConfigurator;
import com.vaadin.flow.di.Lookup;
import com.vaadin.flow.function.DeploymentConfiguration;
import com.vaadin.flow.router.HasErrorParameter;
import com.vaadin.flow.router.Route;
Expand Down Expand Up @@ -375,23 +376,26 @@ public static void initDevModeHandler(Set<Class<?>> classes,
.withEmbeddableWebComponents(true).enablePnpm(enablePnpm)
.withHomeNodeExecRequired(useHomeNodeExec).build();

Lookup lookup = vaadinContext.getAttribute(Lookup.class);

// Check whether executor is provided by the caller (framework)
Object service = config.getInitParameters().get(Executor.class);
Executor service = lookup.lookup(Executor.class);

Runnable runnable = () -> runNodeTasks(vaadinContext, tokenFileData,
tasks);

CompletableFuture<Void> nodeTasksFuture;
if (service instanceof Executor) {
// if there is an executor use it to run the task
nodeTasksFuture = CompletableFuture.runAsync(runnable,
(Executor) service);
} else {
if (service == null) {
nodeTasksFuture = CompletableFuture.runAsync(runnable);

} else {
// if there is an executor use it to run the task
nodeTasksFuture = CompletableFuture.runAsync(runnable, service);
}

DevModeHandler.start(config, builder.npmFolder, nodeTasksFuture);
DevModeHandler.start(
Lookup.compose(lookup,
Lookup.of(config, DeploymentConfiguration.class)),
builder.npmFolder, nodeTasksFuture);
}

/**
Expand Down
Loading

0 comments on commit dedb8dd

Please sign in to comment.