Fix resource leak in Maven plugin

The resource leak increased the number of threads with every execution of
Exclipse-based formatter step in a Maven module. It happened because the
`SpotlessCache` wasn't properly utilized. Every cache key was based on
randomized file names because Maven plugin relocates config files into
the target directory with a random file name. This resulted in all cache
accesses being cache misses.

This commit fixes the problem by:

 * making the Maven plugin use non-random file names for output files.
   `FileLocator` is changed to construct names based on a hash of the
   input path. Input path can be a URL, file in a JAR, or a regular
   local file

 * changing `FileSignature` (used for `SpotlessCache` keys) to use filenames
   instead of paths and file hashes instead of last modified timestamps

These two changes allow `FileSignature` to uniquely identify a set of files
by their content and not take file paths into account. As a result, created
keys for `SpotlessCache` are properly comparable which results in lots of
cache hits and decreased number of created threads.

Changed `FileCignature` only accepts files, not directories. NPM formatters
changed to have their signatures based on package.json file instead of the
whole node_modules directory.

Author: lutovich

lib/src/main/java/com/diffplug/spotless/FileSignature.java

@@ -17,10 +17,14 @@
 
 import static com.diffplug.spotless.MoreIterables.toNullHostileList;
 import static com.diffplug.spotless.MoreIterables.toSortedSet;
+import static java.util.Comparator.comparing;
 
 import java.io.File;
 import java.io.IOException;
 import java.io.Serializable;
+import java.nio.file.Files;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -42,7 +46,7 @@ public final class FileSignature implements Serializable {
 
 	private final String[] filenames;
 	private final long[] filesizes;
-	private final long[] lastModified;
+	private final byte[][] fileHashes;
 
 	/** Method has been renamed to {@link FileSignature#signAsSet}.
 	 * In case no sorting and removal of duplicates is required,
@@ -77,21 +81,21 @@ public static FileSignature signAsSet(File... files) throws IOException {
 
 	/** Creates file signature insensitive to the order of the files. */
 	public static FileSignature signAsSet(Iterable<File> files) throws IOException {
-		return new FileSignature(toSortedSet(files));
+		return new FileSignature(toSortedSet(files, comparing(File::getName)));
 	}
 
 	private FileSignature(final List<File> files) throws IOException {
-		this.files = files;
+		this.files = validateInputFiles(files);
 
 		filenames = new String[this.files.size()];
 		filesizes = new long[this.files.size()];
-		lastModified = new long[this.files.size()];
+		fileHashes = new byte[this.files.size()][];
 
 		int i = 0;
 		for (File file : this.files) {
-			filenames[i] = file.getCanonicalPath();
+			filenames[i] = file.getName();
 			filesizes[i] = file.length();
-			lastModified[i] = file.lastModified();
+			fileHashes[i] = hash(file);
 			++i;
 		}
 	}
@@ -119,4 +123,27 @@ public static String pathNativeToUnix(String pathNative) {
 	public static String pathUnixToNative(String pathUnix) {
 		return LineEnding.nativeIsWin() ? pathUnix.replace('/', '\\') : pathUnix;
 	}
+
+	private static List<File> validateInputFiles(List<File> files) {
+		for (File file : files) {
+			if (!file.isFile()) {
+				throw new IllegalArgumentException(
+						"File signature can only be created for existing regular files, given: "
+								+ file);
+			}
+		}
+		return files;
+	}
+
+	private static byte[] hash(File file) throws IOException {
+		MessageDigest messageDigest;
+		try {
+			messageDigest = MessageDigest.getInstance("SHA-256");
+		} catch (NoSuchAlgorithmException e) {
+			throw new IllegalStateException("SHA-256 digest algorithm not available", e);
+		}
+		byte[] fileContent = Files.readAllBytes(file.toPath());
+		messageDigest.update(fileContent);
+		return messageDigest.digest();
+	}
 }

lib/src/main/java/com/diffplug/spotless/MoreIterables.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 DiffPlug
+ * Copyright 2016-2020 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,14 +20,17 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 
 final class MoreIterables {
 	// prevent direct instantiation
 	private MoreIterables() {}
 
-	/** Returns a shallow copy of input elements, throwing on null elements. */
+	/**
+	 * Returns a shallow copy of input elements, throwing on null elements.
+	 */
 	static <T> List<T> toNullHostileList(Iterable<T> input) {
 		requireElementsNonNull(input);
 		List<T> shallowCopy = (input instanceof Collection)
@@ -37,18 +40,27 @@ static <T> List<T> toNullHostileList(Iterable<T> input) {
 		return shallowCopy;
 	}
 
-	/** Sorts "raw" and removes duplicates, throwing on null elements. */
+	/**
+	 * Sorts "raw" using {@link Comparator#naturalOrder()} and removes duplicates, throwing on null elements.
+	 */
 	static <T extends Comparable<T>> List<T> toSortedSet(Iterable<T> raw) {
+		return toSortedSet(raw, Comparator.naturalOrder());
+	}
+
+	/**
+	 * Sorts "raw" and removes duplicates, throwing on null elements.
+	 */
+	static <T> List<T> toSortedSet(Iterable<T> raw, Comparator<T> comparator) {
 		List<T> toBeSorted = toNullHostileList(raw);
 		// sort it
-		Collections.sort(toBeSorted);
+		Collections.sort(toBeSorted, comparator);
 		// remove any duplicates (normally there won't be any)
 		if (toBeSorted.size() > 1) {
 			Iterator<T> iter = toBeSorted.iterator();
 			T last = iter.next();
 			while (iter.hasNext()) {
 				T next = iter.next();
-				if (next.compareTo(last) == 0) {
+				if (comparator.compare(next, last) == 0) {
 					iter.remove();
 				} else {
 					last = next;

lib/src/main/java/com/diffplug/spotless/npm/NodeServerLayout.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2020 DiffPlug
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.diffplug.spotless.npm;
+
+import java.io.File;
+
+class NodeServerLayout {
+
+	private final File nodeModulesDir;
+	private final File packageJsonFile;
+	private final File serveJsFile;
+
+	NodeServerLayout(File buildDir, String stepName) {
+		this.nodeModulesDir = new File(buildDir, "spotless-node-modules-" + stepName);
+		this.packageJsonFile = new File(nodeModulesDir, "package.json");
+		this.serveJsFile = new File(nodeModulesDir, "serve.js");
+	}
+
+	File nodeModulesDir() {
+		return nodeModulesDir;
+	}
+
+	File packageJsonFile() {
+		return packageJsonFile;
+	}
+
+	File serveJsFile() {
+		return serveJsFile;
+	}
+}

lib/src/main/java/com/diffplug/spotless/npm/NpmFormatterStepStateBase.java

@@ -44,7 +44,7 @@ abstract class NpmFormatterStepStateBase implements Serializable {
 	private static final long serialVersionUID = 1460749955865959948L;
 
 	@SuppressWarnings("unused")
-	private final FileSignature nodeModulesSignature;
+	private final FileSignature packageJsonSignature;
 
 	@SuppressFBWarnings("SE_TRANSIENT_FIELD_NOT_RESTORED")
 	public final transient File nodeModulesDir;
@@ -56,22 +56,26 @@ abstract class NpmFormatterStepStateBase implements Serializable {
 
 	private final String stepName;
 
-	protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, File buildDir, @Nullable File npm) throws IOException {
+	protected NpmFormatterStepStateBase(String stepName, NpmConfig npmConfig, File buildDir,
+			@Nullable File npm) throws IOException {
 		this.stepName = requireNonNull(stepName);
 		this.npmConfig = requireNonNull(npmConfig);
 		this.npmExecutable = resolveNpm(npm);
 
-		this.nodeModulesDir = prepareNodeServer(buildDir);
-		this.nodeModulesSignature = FileSignature.signAsList(this.nodeModulesDir);
+		NodeServerLayout layout = prepareNodeServer(buildDir);
+		this.nodeModulesDir = layout.nodeModulesDir();
+		this.packageJsonSignature = FileSignature.signAsList(layout.packageJsonFile());
 	}
 
-	private File prepareNodeServer(File buildDir) throws IOException {
-		File targetDir = new File(buildDir, "spotless-node-modules-" + stepName);
-		NpmResourceHelper.assertDirectoryExists(targetDir);
-		NpmResourceHelper.writeUtf8StringToFile(targetDir, "package.json", this.npmConfig.getPackageJsonContent());
-		NpmResourceHelper.writeUtf8StringToFile(targetDir, "serve.js", this.npmConfig.getServeScriptContent());
-		runNpmInstall(targetDir);
-		return targetDir;
+	private NodeServerLayout prepareNodeServer(File buildDir) throws IOException {
+		NodeServerLayout layout = new NodeServerLayout(buildDir, stepName);
+		NpmResourceHelper.assertDirectoryExists(layout.nodeModulesDir());
+		NpmResourceHelper.writeUtf8StringToFile(layout.packageJsonFile(),
+				this.npmConfig.getPackageJsonContent());
+		NpmResourceHelper
+				.writeUtf8StringToFile(layout.serveJsFile(), this.npmConfig.getServeScriptContent());
+		runNpmInstall(layout.nodeModulesDir());
+		return layout;
 	}
 
 	private void runNpmInstall(File npmProjectDir) throws IOException {

lib/src/main/java/com/diffplug/spotless/npm/NpmResourceHelper.java

@@ -15,7 +15,11 @@
  */
 package com.diffplug.spotless.npm;
 
-import java.io.*;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.time.Duration;
@@ -24,16 +28,17 @@
 import com.diffplug.spotless.ThrowingEx;
 
 final class NpmResourceHelper {
+
 	private NpmResourceHelper() {
 		// no instance required
 	}
 
-	static void writeUtf8StringToFile(File targetDir, String fileName, String stringToWrite) throws IOException {
-		File packageJsonFile = new File(targetDir, fileName);
-		Files.write(packageJsonFile.toPath(), stringToWrite.getBytes(StandardCharsets.UTF_8));
+	static void writeUtf8StringToFile(File file, String stringToWrite) throws IOException {
+		Files.write(file.toPath(), stringToWrite.getBytes(StandardCharsets.UTF_8));
 	}
 
-	static void writeUtf8StringToOutputStream(String stringToWrite, OutputStream outputStream) throws IOException {
+	static void writeUtf8StringToOutputStream(String stringToWrite, OutputStream outputStream)
+			throws IOException {
 		final byte[] bytes = stringToWrite.getBytes(StandardCharsets.UTF_8);
 		outputStream.write(bytes);
 	}

plugin-maven/src/main/java/com/diffplug/spotless/maven/FileLocator.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 DiffPlug
+ * Copyright 2016-2020 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,10 +16,13 @@
 package com.diffplug.spotless.maven;
 
 import static com.diffplug.common.base.Strings.isNullOrEmpty;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import java.io.File;
+import java.security.MessageDigest;
+import java.security.NoSuchAlgorithmException;
+import java.util.Base64;
 import java.util.Objects;
-import java.util.UUID;
 
 import org.codehaus.plexus.resource.ResourceManager;
 import org.codehaus.plexus.resource.loader.FileResourceCreationException;
@@ -63,16 +66,29 @@ public File locateFile(String path) {
 		}
 	}
 
-	private static String tmpOutputFileName(String path) {
-		String extension = FileUtils.extension(path);
-		return TMP_RESOURCE_FILE_PREFIX + UUID.randomUUID() + '.' + extension;
-	}
-
 	public File getBaseDir() {
 		return baseDir;
 	}
 
 	public File getBuildDir() {
 		return buildDir;
 	}
+
+	private static String tmpOutputFileName(String path) {
+		String extension = FileUtils.extension(path);
+		byte[] pathHash = hash(path);
+		String pathBase64 = Base64.getEncoder().encodeToString(pathHash);
+		return TMP_RESOURCE_FILE_PREFIX + pathBase64 + '.' + extension;
+	}
+
+	private static byte[] hash(String value) {
+		MessageDigest messageDigest;
+		try {
+			messageDigest = MessageDigest.getInstance("SHA-256");
+		} catch (NoSuchAlgorithmException e) {
+			throw new IllegalStateException("SHA-256 digest algorithm not available", e);
+		}
+		messageDigest.update(value.getBytes(UTF_8));
+		return messageDigest.digest();
+	}
 }

testlib/src/test/java/com/diffplug/spotless/FileSignatureTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 DiffPlug
+ * Copyright 2016-2020 DiffPlug
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,11 +16,13 @@
 package com.diffplug.spotless;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 import org.junit.Test;
@@ -48,6 +50,23 @@ public void testFromSet() throws IOException {
 		assertThat(outputFiles).containsExactlyElementsOf(expectedFiles);
 	}
 
+	@Test
+	public void testFromDirectory() {
+		File dir = new File(rootFolder(), "dir");
+		assertThatThrownBy(() -> FileSignature.signAsList(dir))
+				.isInstanceOf(IllegalArgumentException.class);
+	}
+
+	@Test
+	public void testFromFilesAndDirectory() throws IOException {
+		File dir = new File(rootFolder(), "dir");
+		List<File> files = getTestFiles(inputPaths);
+		files.add(dir);
+		Collections.shuffle(files);
+		assertThatThrownBy(() -> FileSignature.signAsList(files))
+				.isInstanceOf(IllegalArgumentException.class);
+	}
+
 	private List<File> getTestFiles(final String[] paths) throws IOException {
 		final List<File> result = new ArrayList<>(paths.length);
 		for (String path : paths) {