1480: allow dynamic npmInstall process

simschla · simschla · commit e97764ff2152 · 2023-02-26T19:42:31.000+01:00
based on cacheDir
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/EslintFormatterStep.java
@@ -71,13 +71,13 @@ public static Map<String, String> defaultDevDependenciesWithEslint(String versio
 		return Collections.singletonMap("eslint", version);
 	}
 
-	public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) {
+	public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) {
 		requireNonNull(devDependencies);
 		requireNonNull(provisioner);
 		requireNonNull(projectDir);
 		requireNonNull(buildDir);
 		return FormatterStep.createLazy(NAME,
-				() -> new State(NAME, devDependencies, projectDir, buildDir, npmPathResolver, eslintConfig),
+				() -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, eslintConfig),
 				State::createFormatterFunc);
 	}
 
@@ -89,7 +89,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 		@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
 		private transient EslintConfig eslintConfigInUse;
 
-		State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException {
+		State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, EslintConfig eslintConfig) throws IOException {
 			super(stepName,
 					new NpmConfig(
 							replaceDevDependencies(
@@ -102,6 +102,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 					new NpmFormatterStepLocations(
 							projectDir,
 							buildDir,
+							cacheDir,
 							npmPathResolver::resolveNpmExecutable,
 							npmPathResolver::resolveNodeExecutable));
 			this.origEslintConfig = requireNonNull(eslintConfig.verify());
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeApp.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeApp.java
@@ -42,13 +42,22 @@ public class NodeApp {
 	@Nonnull
 	protected final NpmFormatterStepLocations formatterStepLocations;
 
-	public NodeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmProcessFactory npmProcessFactory, @Nonnull NpmFormatterStepLocations formatterStepLocations) {
+	public NodeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations formatterStepLocations) {
 		this.nodeServerLayout = Objects.requireNonNull(nodeServerLayout);
 		this.npmConfig = Objects.requireNonNull(npmConfig);
-		this.npmProcessFactory = Objects.requireNonNull(npmProcessFactory);
+		this.npmProcessFactory = processFactory(formatterStepLocations);
 		this.formatterStepLocations = Objects.requireNonNull(formatterStepLocations);
 	}
 
+	private static NpmProcessFactory processFactory(NpmFormatterStepLocations formatterStepLocations) {
+		if (formatterStepLocations.cacheDir() != null) {
+			logger.info("Caching npm install results in {}.", formatterStepLocations.cacheDir());
+			return NodeModulesCachingNpmProcessFactory.create(formatterStepLocations.cacheDir());
+		}
+		logger.debug("Not caching npm install results.");
+		return StandardNpmProcessFactory.INSTANCE;
+	}
+
 	boolean needsNpmInstall() {
 		return !this.nodeServerLayout.isNodeModulesPrepared();
 	}
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeModulesCachingNpmProcessFactory.java
@@ -17,6 +17,9 @@
 
 import java.io.File;
 import java.util.List;
+import java.util.Objects;
+
+import javax.annotation.Nonnull;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,8 +37,8 @@ public class NodeModulesCachingNpmProcessFactory implements NpmProcessFactory {
 
 	private final ShadowCopy shadowCopy;
 
-	private NodeModulesCachingNpmProcessFactory(File cacheDir) {
-		this.cacheDir = cacheDir;
+	private NodeModulesCachingNpmProcessFactory(@Nonnull File cacheDir) {
+		this.cacheDir = Objects.requireNonNull(cacheDir);
 		assertDir(cacheDir);
 		this.shadowCopy = new ShadowCopy(cacheDir);
 	}
@@ -51,7 +54,7 @@ private void assertDir(File cacheDir) {
 		}
 	}
 
-	public static NodeModulesCachingNpmProcessFactory forCacheDir(File cacheDir) {
+	public static NodeModulesCachingNpmProcessFactory create(@Nonnull File cacheDir) {
 		return new NodeModulesCachingNpmProcessFactory(cacheDir);
 	}
 
@@ -87,13 +90,16 @@ public Result waitFor() {
 				Result result = timedLogger.withInfo("calling actual npm install {}", actualNpmInstallProcess.describe())
 						.call(actualNpmInstallProcess::waitFor);
 				assert result.exitCode() == 0;
-				// TODO: maybe spawn a thread to do this in the background?
-				timedLogger.withInfo("Caching node_modules for {} in {}", entryName, cacheDir)
-						.run(() -> shadowCopy.addEntry(entryName(), new File(nodeServerLayout.nodeModulesDir(), NodeServerLayout.NODE_MODULES)));
+				storeShadowCopy(entryName);
 				return result;
 			}
 		}
 
+		private void storeShadowCopy(String entryName) {
+			timedLogger.withInfo("Caching node_modules for {} in {}", entryName, cacheDir)
+					.run(() -> shadowCopy.addEntry(entryName(), new File(nodeServerLayout.nodeModulesDir(), NodeServerLayout.NODE_MODULES)));
+		}
+
 		private String entryName() {
 			return nodeServerLayout.nodeModulesDir().getName();
 		}
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java b/lib/src/main/java/com/diffplug/spotless/npm/NodeServeApp.java
@@ -29,8 +29,8 @@ public class NodeServeApp extends NodeApp {
 
 	private static final TimedLogger timedLogger = TimedLogger.forLogger(logger);
 
-	public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmProcessFactory npmProcessFactory, @Nonnull NpmFormatterStepLocations formatterStepLocations) {
-		super(nodeServerLayout, npmConfig, npmProcessFactory, formatterStepLocations);
+	public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations formatterStepLocations) {
+		super(nodeServerLayout, npmConfig, formatterStepLocations);
 	}
 
 	ProcessRunner.LongRunningProcess startNpmServeProcess() {
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepLocations.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepLocations.java
@@ -21,6 +21,8 @@
 import java.io.Serializable;
 import java.util.function.Supplier;
 
+import javax.annotation.Nonnull;
+
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 class NpmFormatterStepLocations implements Serializable {
@@ -32,15 +34,19 @@ class NpmFormatterStepLocations implements Serializable {
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
 	private final transient File buildDir;
 
+	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
+	private final transient File cacheDir;
+
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
 	private final transient Supplier<File> npmExecutable;
 
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
 	private final transient Supplier<File> nodeExecutable;
 
-	public NpmFormatterStepLocations(File projectDir, File buildDir, Supplier<File> npmExecutable, Supplier<File> nodeExecutable) {
+	public NpmFormatterStepLocations(@Nonnull File projectDir, @Nonnull File buildDir, File cacheDir, @Nonnull Supplier<File> npmExecutable, @Nonnull Supplier<File> nodeExecutable) {
 		this.projectDir = requireNonNull(projectDir);
 		this.buildDir = requireNonNull(buildDir);
+		this.cacheDir = cacheDir;
 		this.npmExecutable = requireNonNull(npmExecutable);
 		this.nodeExecutable = requireNonNull(nodeExecutable);
 	}
@@ -53,6 +59,10 @@ public File buildDir() {
 		return buildDir;
 	}
 
+	public File cacheDir() {
+		return cacheDir;
+	}
+
 	public File npmExecutable() {
 		return npmExecutable.get();
 	}
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java b/lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java
@@ -62,7 +62,7 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, NpmFor
 		this.npmConfig = requireNonNull(npmConfig);
 		this.locations = locations;
 		this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), npmConfig.getPackageJsonContent());
-		this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, NodeModulesCachingNpmProcessFactory.forCacheDir(new File(locations.buildDir(), "spotless-npm-cache"))/*StandardNpmProcessFactory.INSTANCE*/, locations);
+		this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, locations);
 	}
 
 	protected void prepareNodeServerLayout() throws IOException {
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/PrettierFormatterStep.java
@@ -50,12 +50,12 @@ public static final Map<String, String> defaultDevDependenciesWithPrettier(Strin
 		return Collections.singletonMap("prettier", version);
 	}
 
-	public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) {
+	public static FormatterStep create(Map<String, String> devDependencies, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) {
 		requireNonNull(devDependencies);
 		requireNonNull(provisioner);
 		requireNonNull(buildDir);
 		return FormatterStep.createLazy(NAME,
-				() -> new State(NAME, devDependencies, projectDir, buildDir, npmPathResolver, prettierConfig),
+				() -> new State(NAME, devDependencies, projectDir, buildDir, cacheDir, npmPathResolver, prettierConfig),
 				State::createFormatterFunc);
 	}
 
@@ -64,7 +64,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 		private static final long serialVersionUID = -539537027004745812L;
 		private final PrettierConfig prettierConfig;
 
-		State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) throws IOException {
+		State(String stepName, Map<String, String> devDependencies, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, PrettierConfig prettierConfig) throws IOException {
 			super(stepName,
 					new NpmConfig(
 							replaceDevDependencies(
@@ -77,6 +77,7 @@ private static class State extends NpmFormatterStepStateBase implements Serializ
 					new NpmFormatterStepLocations(
 							projectDir,
 							buildDir,
+							cacheDir,
 							npmPathResolver::resolveNpmExecutable,
 							npmPathResolver::resolveNodeExecutable));
 			this.prettierConfig = requireNonNull(prettierConfig);
diff --git a/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java b/lib/src/main/java/com/diffplug/spotless/npm/TsFmtFormatterStep.java
@@ -40,11 +40,11 @@ public class TsFmtFormatterStep {
 
 	public static final String NAME = "tsfmt-format";
 
-	public static FormatterStep create(Map<String, String> versions, Provisioner provisioner, File projectDir, File buildDir, NpmPathResolver npmPathResolver, @Nullable TypedTsFmtConfigFile configFile, @Nullable Map<String, Object> inlineTsFmtSettings) {
+	public static FormatterStep create(Map<String, String> versions, Provisioner provisioner, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, @Nullable TypedTsFmtConfigFile configFile, @Nullable Map<String, Object> inlineTsFmtSettings) {
 		requireNonNull(provisioner);
 		requireNonNull(buildDir);
 		return FormatterStep.createLazy(NAME,
-				() -> new State(NAME, versions, projectDir, buildDir, npmPathResolver, configFile, inlineTsFmtSettings),
+				() -> new State(NAME, versions, projectDir, buildDir, cacheDir, npmPathResolver, configFile, inlineTsFmtSettings),
 				State::createFormatterFunc);
 	}
 
@@ -71,7 +71,7 @@ public static class State extends NpmFormatterStepStateBase implements Serializa
 		@Nullable
 		private final TypedTsFmtConfigFile configFile;
 
-		public State(String stepName, Map<String, String> versions, File projectDir, File buildDir, NpmPathResolver npmPathResolver, @Nullable TypedTsFmtConfigFile configFile, @Nullable Map<String, Object> inlineTsFmtSettings) throws IOException {
+		public State(String stepName, Map<String, String> versions, File projectDir, File buildDir, File cacheDir, NpmPathResolver npmPathResolver, @Nullable TypedTsFmtConfigFile configFile, @Nullable Map<String, Object> inlineTsFmtSettings) throws IOException {
 			super(stepName,
 					new NpmConfig(
 							replaceDevDependencies(NpmResourceHelper.readUtf8StringFromClasspath(TsFmtFormatterStep.class, "/com/diffplug/spotless/npm/tsfmt-package.json"), new TreeMap<>(versions)),
@@ -82,6 +82,7 @@ public State(String stepName, Map<String, String> versions, File projectDir, Fil
 					new NpmFormatterStepLocations(
 							projectDir,
 							buildDir,
+							cacheDir,
 							npmPathResolver::resolveNpmExecutable,
 							npmPathResolver::resolveNodeExecutable));
 			this.buildDir = requireNonNull(buildDir);
diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/EslintFormatterStepTest.java
@@ -64,6 +64,7 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new EslintConfig(eslintRc, null));
 
@@ -107,6 +108,7 @@ void formattingUsingRulesetsFile(String ruleSetName) throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new EslintTypescriptConfig(eslintRc, null, tsconfigFile));
 
@@ -164,6 +166,7 @@ void formattingUsingInlineXoConfig() throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new EslintTypescriptConfig(null, esLintConfig, tsconfigFile));
 
diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/NpmFormatterStepCommonTests.java b/testlib/src/test/java/com/diffplug/spotless/npm/NpmFormatterStepCommonTests.java
@@ -56,4 +56,13 @@ protected File projectDir() throws IOException {
 		}
 		return this.projectDir;
 	}
+
+	private File cacheDir = null;
+
+	protected File cacheDir() throws IOException {
+		if (this.cacheDir == null) {
+			this.cacheDir = newFolder("cache-dir");
+		}
+		return this.cacheDir;
+	}
 }
diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/PrettierFormatterStepTest.java
@@ -52,6 +52,7 @@ void formattingUsingConfigFile(String fileType) throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new PrettierConfig(prettierRc, null));
 
@@ -77,6 +78,7 @@ void parserInferenceBasedOnExplicitFilepathIsWorking() throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new PrettierConfig(null, ImmutableMap.of("filepath", "anyname.json"))); // should select parser based on this name
 
@@ -97,6 +99,7 @@ void parserInferenceBasedOnFilenameIsWorking() throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new PrettierConfig(null, Collections.emptyMap()));
 
@@ -112,6 +115,7 @@ void verifyPrettierErrorMessageIsRelayed() throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					new PrettierConfig(null, ImmutableMap.of("parser", "postcss")));
 			try (StepHarnessWithFile stepHarness = StepHarnessWithFile.forStep(this, formatterStep)) {
@@ -137,6 +141,7 @@ void runFormatTest(PrettierConfig config, String cleanFileNameSuffix) throws Exc
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					config); // should select parser based on this name
 
diff --git a/testlib/src/test/java/com/diffplug/spotless/npm/TsFmtFormatterStepTest.java b/testlib/src/test/java/com/diffplug/spotless/npm/TsFmtFormatterStepTest.java
@@ -59,6 +59,7 @@ void formattingUsingConfigFile(String formattingConfigFile) throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					TypedTsFmtConfigFile.named(configFileNameWithoutExtension, configFile),
 					Collections.emptyMap());
@@ -82,6 +83,7 @@ void formattingUsingInlineConfigWorks() throws Exception {
 					TestProvisioner.mavenCentral(),
 					projectDir(),
 					buildDir(),
+					cacheDir(),
 					npmPathResolver(),
 					null,
 					inlineConfig);

Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,8 @@ public class NodeServeApp extends NodeApp {`
`29`	`29`
`30`	`30`	`private static final TimedLogger timedLogger = TimedLogger.forLogger(logger);`
`31`	`31`
`32`		`- public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmProcessFactory npmProcessFactory, @Nonnull NpmFormatterStepLocations formatterStepLocations) {`
`33`		`- super(nodeServerLayout, npmConfig, npmProcessFactory, formatterStepLocations);`
	`32`	`+ public NodeServeApp(@Nonnull NodeServerLayout nodeServerLayout, @Nonnull NpmConfig npmConfig, @Nonnull NpmFormatterStepLocations formatterStepLocations) {`
	`33`	`+ super(nodeServerLayout, npmConfig, formatterStepLocations);`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`ProcessRunner.LongRunningProcess startNpmServeProcess() {`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, NpmFor`
`62`	`62`	`this.npmConfig = requireNonNull(npmConfig);`
`63`	`63`	`this.locations = locations;`
`64`	`64`	`this.nodeServerLayout = new NodeServerLayout(locations.buildDir(), npmConfig.getPackageJsonContent());`
`65`		`- this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, NodeModulesCachingNpmProcessFactory.forCacheDir(new File(locations.buildDir(), "spotless-npm-cache"))/StandardNpmProcessFactory.INSTANCE/, locations);`
	`65`	`+ this.nodeServeApp = new NodeServeApp(nodeServerLayout, npmConfig, locations);`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`protected void prepareNodeServerLayout() throws IOException {`
Original file line number	Diff line number	Diff line change
`@@ -56,4 +56,13 @@ protected File projectDir() throws IOException {`
`56`	`56`	`}`
`57`	`57`	`return this.projectDir;`
`58`	`58`	`}`
	`59`	`+`
	`60`	`+ private File cacheDir = null;`
	`61`	`+`
	`62`	`+ protected File cacheDir() throws IOException {`
	`63`	`+ if (this.cacheDir == null) {`
	`64`	`+ this.cacheDir = newFolder("cache-dir");`
	`65`	`+ }`
	`66`	`+ return this.cacheDir;`
	`67`	`+ }`
`59`	`68`	`}`