Skip to content

Commit

Permalink
fix: use global node and tools if they are available first (#10753) (#…
Browse files Browse the repository at this point in the history 
…10769)

fixes #9863

Co-authored-by: Denis <denis@vaadin.com>
  • Loading branch information
@vaadin-bot
vaadin-bot and Denis committed Apr 29, 2021
1 parent 0322e23 commit d7b59dd
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 49 deletions.
8 changes: 4 additions & 4 deletions flow-server/src/main/java/com/vaadin/flow/server/DevModeHandler.java
View file
Expand Up @@ -659,12 +659,12 @@ private boolean doStartWebpack(ApplicationConfiguration config,
ProcessBuilder processBuilder = new ProcessBuilder()
.directory(npmFolder);

FrontendTools tools = new FrontendTools(npmFolder.getAbsolutePath(),
() -> FrontendUtils.getVaadinHomeDirectory().getAbsolutePath());
tools.validateNodeAndNpmVersion();

boolean useHomeNodeExec = config.getBooleanProperty(
InitParameters.REQUIRE_HOME_NODE_EXECUTABLE, false);
FrontendTools tools = new FrontendTools(npmFolder.getAbsolutePath(),
() -> FrontendUtils.getVaadinHomeDirectory().getAbsolutePath(),
useHomeNodeExec);
tools.validateNodeAndNpmVersion();

String nodeExec = null;
if (useHomeNodeExec) {
Expand Down
150 changes: 117 additions & 33 deletions flow-server/src/main/java/com/vaadin/flow/server/frontend/FrontendTools.java
View file
Expand Up @@ -25,6 +25,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Properties;
import java.util.function.Supplier;
import java.util.stream.Stream;
Expand Down Expand Up @@ -154,6 +155,33 @@ String getScript() {

private final boolean ignoreVersionChecks;

private final boolean forceAlternativeNode;

/**
* Creates an instance of the class using the {@code baseDir} as a base
* directory to locate the tools and the directory returned by the
* {@code alternativeDirGetter} as a directory to install tools if they are
* not found and use it as an alternative tools location.
* <p>
* If {@code alternativeDir} is {@code null} tools won't be installed.
*
*
* @param baseDir
* the base directory to locate the tools, not {@code null}
* @param alternativeDirGetter
* the getter for a directory where tools will be installed if
* they are not found globally or in the {@code baseDir}, may be
* {@code null}
* @param forceAlternativeNode
* force usage of node executable from alternative directory
*/
public FrontendTools(String baseDir, Supplier<String> alternativeDirGetter,
boolean forceAlternativeNode) {
this(baseDir, alternativeDirGetter, DEFAULT_NODE_VERSION,
URI.create(NodeInstaller.DEFAULT_NODEJS_DOWNLOAD_ROOT),
forceAlternativeNode);
}

/**
* Creates an instance of the class using the {@code baseDir} as a base
* directory to locate the tools and the directory returned by the
Expand All @@ -173,7 +201,7 @@ String getScript() {
public FrontendTools(String baseDir,
Supplier<String> alternativeDirGetter) {
this(baseDir, alternativeDirGetter, DEFAULT_NODE_VERSION,
URI.create(NodeInstaller.DEFAULT_NODEJS_DOWNLOAD_ROOT));
URI.create(NodeInstaller.DEFAULT_NODEJS_DOWNLOAD_ROOT), false);
}

/**
Expand All @@ -200,22 +228,58 @@ public FrontendTools(String baseDir,
* corporate environments where the node.js download can be
* provided from an intranet mirror. Use
* {@link NodeInstaller#DEFAULT_NODEJS_DOWNLOAD_ROOT} by default.
* @param forceAlternativeNode
* force usage of node executable from alternative directory
*/
public FrontendTools(String baseDir, Supplier<String> alternativeDirGetter,
String nodeVersion, URI nodeDownloadRoot) {
String nodeVersion, URI nodeDownloadRoot,
boolean forceAlternativeNode) {
this(baseDir, alternativeDirGetter, nodeVersion, nodeDownloadRoot,
"true".equalsIgnoreCase(System.getProperty(
FrontendUtils.PARAM_IGNORE_VERSION_CHECKS)));
FrontendUtils.PARAM_IGNORE_VERSION_CHECKS)),
forceAlternativeNode);
}

/**
* Creates an instance of the class using the {@code baseDir} as a base
* directory to locate the tools and the directory returned by the
* {@code alternativeDirGetter} as a directory to install tools if they are
* not found and use it as an alternative tools location.
* <p>
* If {@code alternativeDir} is {@code null} tools won't be installed.
*
*
* @param baseDir
* the base directory to locate the tools, not {@code null}
* @param alternativeDirGetter
* the getter for a directory where tools will be installed if
* they are not found globally or in the {@code baseDir}, may be
* {@code null}
* @param nodeVersion
* The node.js version to be used when node.js is installed
* automatically by Vaadin, for example <code>"v14.15.4"</code>.
* Use {@value #DEFAULT_NODE_VERSION} by default.
* @param nodeDownloadRoot
* Download node.js from this URL. Handy in heavily firewalled
* corporate environments where the node.js download can be
* provided from an intranet mirror. Use
* {@link NodeInstaller#DEFAULT_NODEJS_DOWNLOAD_ROOT} by default.
*/
public FrontendTools(String baseDir, Supplier<String> alternativeDirGetter,
String nodeVersion, URI nodeDownloadRoot) {
this(baseDir, alternativeDirGetter, nodeVersion, nodeDownloadRoot,
false);
}

FrontendTools(String baseDir, Supplier<String> alternativeDirGetter,
String nodeVersion, URI nodeDownloadRoot,
boolean ignoreVersionChecks) {
boolean ignoreVersionChecks, boolean forceAlternativeNode) {
this.baseDir = Objects.requireNonNull(baseDir);
this.alternativeDirGetter = alternativeDirGetter;
this.nodeVersion = Objects.requireNonNull(nodeVersion);
this.nodeDownloadRoot = Objects.requireNonNull(nodeDownloadRoot);
this.ignoreVersionChecks = ignoreVersionChecks;
this.forceAlternativeNode = forceAlternativeNode;
}

/**
Expand All @@ -225,8 +289,23 @@ public FrontendTools(String baseDir, Supplier<String> alternativeDirGetter,
*/
public String getNodeExecutable() {
Pair<String, String> nodeCommands = getNodeCommands();
return getExecutable(nodeCommands.getFirst(), nodeCommands.getSecond(),
alternativeDirGetter != null).getAbsolutePath();
File file = getExecutable(baseDir, nodeCommands.getSecond());
if (file == null) {
file = frontendToolsLocator.tryLocateTool(nodeCommands.getFirst())
.orElse(null);
}
if (file == null) {
file = getExecutable(getAlternativeDir(), nodeCommands.getSecond());
}
if (file == null && alternativeDirGetter != null) {
getLogger().info("Couldn't find {}. Installing Node and NPM to {}.",
nodeCommands.getFirst(), getAlternativeDir());
file = new File(installNode(nodeVersion, nodeDownloadRoot));
}
if (file == null) {
throw new IllegalStateException(String.format(NODE_NOT_FOUND));
}
return file.getAbsolutePath();
}

/**
Expand Down Expand Up @@ -299,7 +378,7 @@ public void validateNodeAndNpmVersion() {
}
try {
List<String> nodeVersionCommand = new ArrayList<>();
nodeVersionCommand.add(getNodeExecutable());
nodeVersionCommand.add(doGetNodeExecutable());
nodeVersionCommand.add("--version"); // NOSONAR
FrontendVersion foundNodeVersion = FrontendUtils.getVersion("node",
nodeVersionCommand);
Expand Down Expand Up @@ -385,28 +464,12 @@ void checkForFaultyNpmVersion(FrontendVersion npmVersion) {
}
}

private File getExecutable(String cmd, String defaultLocation,
boolean installNode) {
File file = null;
if (defaultLocation == null) {
file = frontendToolsLocator.tryLocateTool(cmd).orElse(null);
} else {
file = Arrays.asList(() -> baseDir, alternativeDirGetter).stream()
.map(Supplier::get)
.map(dir -> new File(dir, defaultLocation))
.filter(frontendToolsLocator::verifyTool).findFirst()
.orElseGet(() -> frontendToolsLocator.tryLocateTool(cmd)
.orElse(null));
}
if (file == null && installNode) {
getLogger().info("Couldn't find {}. Installing Node and NPM to {}.",
cmd, getAlternativeDir());
return new File(installNode(nodeVersion, nodeDownloadRoot));
private File getExecutable(String dir, String location) {
File file = new File(dir, location);
if (frontendToolsLocator.verifyTool(file)) {
return file;
}
if (file == null) {
throw new IllegalStateException(String.format(NODE_NOT_FOUND));
}
return file;
return null;
}

private Pair<String, String> getNodeCommands() {
Expand Down Expand Up @@ -550,17 +613,30 @@ private List<String> getNpmCliToolExecutable(NpmCliTool cliTool,
// First look for *-cli.js script in project/node_modules
List<String> returnCommand = getNpmScriptCommand(baseDir,
cliTool.getScript());
if (returnCommand.isEmpty()) {
boolean alternativeDirChecked = false;
if (returnCommand.isEmpty() && forceAlternativeNode) {
// First look for *-cli.js script in ~/.vaadin/node/node_modules
// only if alternative node takes precedence over all other location
returnCommand = getNpmScriptCommand(getAlternativeDir(),
cliTool.getScript());
alternativeDirChecked = true;
}
if (returnCommand.isEmpty()) {
// Otherwise look for regular `npm`/`npx` global search path
returnCommand = Collections.singletonList(
getExecutable(cliTool.getCommand(), null, true)
.getAbsolutePath());
Optional<String> command = frontendToolsLocator
.tryLocateTool(cliTool.getCommand())
.map(File::getAbsolutePath);
if (command.isPresent()) {
returnCommand = Collections.singletonList(command.get());
}
}
if (!alternativeDirChecked && returnCommand.isEmpty()) {
// Use alternative if global is not found and alternative location
// is not yet checked
returnCommand = getNpmScriptCommand(getAlternativeDir(),
cliTool.getScript());
}

if (flags.length > 0) {
returnCommand = new ArrayList<>(returnCommand);
Collections.addAll(returnCommand, flags);
Expand All @@ -576,7 +652,7 @@ private List<String> getNpmScriptCommand(String dir, String scriptName) {
List<String> returnCommand = new ArrayList<>();
if (file.canRead()) {
// We return a two element list with node binary and npm-cli script
returnCommand.add(getNodeExecutable());
returnCommand.add(doGetNodeExecutable());
returnCommand.add(file.getAbsolutePath());
}
return returnCommand;
Expand Down Expand Up @@ -639,4 +715,12 @@ private String buildBadVersionString(String tool, String version,
private String getAlternativeDir() {
return alternativeDirGetter.get();
}

private String doGetNodeExecutable() {
if (forceAlternativeNode) {
return forceAlternativeNodeExecutable();
} else {
return getNodeExecutable();
}
}
}
2 changes: 1 addition & 1 deletion flow-server/src/main/java/com/vaadin/flow/server/frontend/TaskRunNpmInstall.java
View file
Expand Up @@ -375,7 +375,7 @@ private void runNpmInstall() throws ExecutionFailedException {

FrontendTools tools = new FrontendTools(baseDir,
() -> FrontendUtils.getVaadinHomeDirectory().getAbsolutePath(),
nodeVersion, nodeDownloadRoot);
nodeVersion, nodeDownloadRoot, requireHomeNodeExec);
try {
if (requireHomeNodeExec) {
tools.forceAlternativeNodeExecutable();
Expand Down
22 changes: 11 additions & 11 deletions flow-server/src/test/java/com/vaadin/flow/server/frontend/FrontendToolsTest.java
View file
Expand Up @@ -190,12 +190,17 @@ public void installNodeFromFileSystem_ForceAlternativeNodeExecutableInstallsToTa
}

@Test
public void should_useSystemNode() {
public void homeNodeIsNotForced_useGlobalNode() throws IOException {
createStubNode(true, true, vaadinHomeDir);

assertThat(tools.getNodeExecutable(), containsString("node"));
assertThat(tools.getNodeExecutable(),
not(containsString(DEFAULT_NODE)));
assertThat(tools.getNodeExecutable(),
not(containsString(NPM_CLI_STRING)));
assertThat(tools.getNodeExecutable(),
not(containsString(vaadinHomeDir)));
assertThat(tools.getNodeExecutable(), not(containsString(baseDir)));

assertEquals(4, tools.getNpmExecutable().size());
assertThat(tools.getNpmExecutable().get(0), containsString("npm"));
Expand Down Expand Up @@ -433,14 +438,6 @@ public void should_useProjectNodeFirst() throws Exception {
assertNodeCommand(() -> baseDir);
}

@Test
public void should_useHomeFirst() throws Exception {
Assume.assumeFalse(
"Skipping test on windows until a fake node.exe that isn't caught by Window defender can be created.",
FrontendUtils.isWindows());
assertNodeCommand(() -> vaadinHomeDir);
}

@Test
public void should_useProjectNpmFirst() throws Exception {
Assume.assumeFalse(
Expand All @@ -452,10 +449,13 @@ public void should_useProjectNpmFirst() throws Exception {
}

@Test
public void should_useHomeNpmFirst() throws Exception {
public void forceHomeNode_useHomeNpmFirst() throws Exception {
Assume.assumeFalse(
"Skipping test on windows until a fake node.exe that isn't caught by Window defender can be created.",
FrontendUtils.isWindows());
tools = new FrontendTools(baseDir, () -> vaadinHomeDir, true);

createStubNode(true, true, vaadinHomeDir);
assertNpmCommand(() -> vaadinHomeDir);
}

Expand Down Expand Up @@ -485,7 +485,7 @@ public void getSuitablePnpm_tooOldVersionInstalledAndSkipVersionCheck_accepted()
throws Exception {
tools = new FrontendTools(baseDir, () -> vaadinHomeDir,
FrontendTools.DEFAULT_NODE_VERSION, new File(baseDir).toURI(),
true);
true, false);
Assume.assumeFalse(tools.getNodeExecutable().isEmpty());
createStubNode(false, true, baseDir);
createFakePnpm("4.5.0");
Expand Down

0 comments on commit d7b59dd

Please sign in to comment.