Improve ProcessHelper ergonomics

at the moment the timeout is removed, but it probably isn't needed anymore

Author: MartelliEnrico

src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java

@@ -138,6 +138,8 @@ private void answerFile(TelegramRequest request, String fileId) {
 			}
 		} catch (TelegramApiException | MediaException e) {
 			processFailure(request, e);
+		} catch (InterruptedException e) {
+			Thread.currentThread().interrupt();
 		} finally {
 			deleteTempFiles(pathsToDelete);
 		}

src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java

@@ -63,8 +63,9 @@ public final class MediaHelper {
 	 * @param inputFile the file to convert
 	 * @return a resized and converted file
 	 * @throws MediaException if the file is not supported or if the conversion failed
+	 * @throws InterruptedException if the current thread is interrupted while converting a video file
 	 */
-	public static File convert(File inputFile) throws MediaException {
+	public static File convert(File inputFile) throws MediaException, InterruptedException {
 		var mimeType = detectMimeType(inputFile);
 
 		try {
@@ -331,8 +332,9 @@ private static void deleteFile(File file) throws FileOperationException {
 	 * @param file the file to convert
 	 * @return converted video, {@code null} if no conversion was needed
 	 * @throws MediaException if file conversion is not successful
+	 * @throws InterruptedException if the current thread is interrupted while converting the video file
 	 */
-	private static File convertToWebm(File file) throws MediaException {
+	private static File convertToWebm(File file) throws MediaException, InterruptedException {
 		if (isVideoCompliant(file)) {
 			LOGGER.atInfo().log("The video doesn't need conversion");
 
@@ -386,8 +388,9 @@ private static MultimediaInfo retrieveMultimediaInfo(File file) throws Corrupted
 	 * @param file the file to convert
 	 * @return converted video
 	 * @throws MediaException if file conversion is not successful
+	 * @throws InterruptedException if the current thread is interrupted while converting the file
 	 */
-	private static File convertWithFfmpeg(File file) throws MediaException {
+	private static File convertWithFfmpeg(File file) throws MediaException, InterruptedException {
 		var webmVideo = createTempFile("webm");
 		var logPrefix = webmVideo.getAbsolutePath() + "-passlog";
 		var baseCommand = new String[] {
@@ -413,9 +416,13 @@ private static File convertWithFfmpeg(File file) throws MediaException {
 			} catch (FileOperationException ex) {
 				e.addSuppressed(ex);
 			}
-			throw new MediaException(e.getMessage());
+			throw new MediaException(e);
 		} finally {
-			deleteLogFile(logPrefix);
+			try {
+				deleteFile(new File(logPrefix + "-0.log"));
+			} catch (FileOperationException _) {
+				// ignore the exception
+			}
 		}
 
 		return webmVideo;
@@ -435,18 +442,6 @@ private static String[] buildFfmpegCommand(String[] baseCommand, String... addit
 		return commands;
 	}
 
-	/**
-	 * Delete the passlog file based on the prefix
-	 *
-	 * @param prefix the prefix of the passlog file
-	 */
-	private static void deleteLogFile(String prefix) {
-		try {
-			deleteFile(new File(prefix + "-0.log"));
-		} catch (FileOperationException _) {
-		}
-	}
-
 	private MediaHelper() {
 		throw new UnsupportedOperationException();
 	}

src/main/java/com/github/stickerifier/stickerify/process/PathLocator.java

@@ -1,7 +1,6 @@
 package com.github.stickerifier.stickerify.process;
 
 import static com.github.stickerifier.stickerify.process.ProcessHelper.IS_WINDOWS;
-import static java.lang.System.lineSeparator;
 
 import com.github.stickerifier.stickerify.exception.ProcessException;
 import org.slf4j.Logger;
@@ -24,12 +23,14 @@ public class PathLocator implements ProcessLocator {
 	public PathLocator() {
 		try {
 			if (ffmpegLocation == null || ffmpegLocation.isBlank()) {
-				ffmpegLocation = ProcessHelper.executeCommand(FIND_FFMPEG).split(lineSeparator())[0];
+				ffmpegLocation = ProcessHelper.executeCommand(FIND_FFMPEG).getFirst();
 			}
 
 			LOGGER.atInfo().log("FFmpeg is installed at {}", ffmpegLocation);
 		} catch (ProcessException e) {
 			LOGGER.atError().setCause(e).log("Unable to detect FFmpeg's installation path");
+		} catch (InterruptedException _) {
+			Thread.currentThread().interrupt();
 		}
 	}
 

src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java

@@ -1,12 +1,12 @@
 package com.github.stickerifier.stickerify.process;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.concurrent.TimeUnit.MINUTES;
 
 import com.github.stickerifier.stickerify.exception.ProcessException;
 
 import java.io.IOException;
-import java.io.InputStream;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.concurrent.Semaphore;
 
 public final class ProcessHelper {
@@ -21,65 +21,40 @@ public final class ProcessHelper {
 	 * on Windows they will be at most 4, on every other system they will be at most 5.
 	 *
 	 * @param command the command to be executed
-	 * @return the output of the command
+	 * @return the output of the command, split by lines
 	 * @throws ProcessException either if:
 	 * <ul>
 	 *     <li>the command was unsuccessful
-	 *     <li>the waiting time elapsed
 	 *     <li>an unexpected failure happened running the command
+	 *     <li>an unexpected failure happened reading the output
 	 * </ul>
+	 * @throws InterruptedException if the current thread is interrupted while waiting for the command to finish execution
 	 */
-	public static String executeCommand(final String[] command) throws ProcessException {
-		Process process = null;
-
+	public static List<String> executeCommand(final String[] command) throws ProcessException, InterruptedException {
 		try {
 			SEMAPHORE.acquire();
-			process = new ProcessBuilder(command).start();
-			var processExited = process.waitFor(1, MINUTES);
+			var process = new ProcessBuilder(command).redirectErrorStream(true).start();
 
-			if (!processExited || process.exitValue() != 0) {
-				var reason = processExited ? "successfully" : "in time";
-				var output = readStream(process.getErrorStream());
-				throw new ProcessException("The command {} couldn't complete {}\n{}", command[0], reason, output);
+			var output = new LinkedList<String>();
+			try (var reader = process.inputReader(UTF_8)) {
+				reader.lines().forEach(output::add);
 			}
 
-			return readProcessOutput(process);
-		} catch (IOException | InterruptedException e) {
+			var exitCode = process.waitFor();
+			if (exitCode != 0) {
+				process.destroy();
+				var lines = String.join("\n", output);
+				throw new ProcessException("The command {} exited with code {}\n{}", command[0], exitCode, lines);
+			}
+
+			return output;
+		} catch (IOException e) {
 			throw new ProcessException(e);
 		} finally {
 			SEMAPHORE.release();
-			if (process != null) {
-				process.destroy();
-			}
 		}
 	}
 
-	/**
-	 * Processes the content of the stream and retrieves its UTF-8 string representation.
-	 *
-	 * @param stream the stream to be decoded
-	 * @return the UTF-8 representation of passed-in stream
-	 * @throws IOException if an error occurs reading stream's bytes
-	 */
-	private static String readStream(InputStream stream) throws IOException {
-		return new String(stream.readAllBytes(), UTF_8);
-	}
-
-	/**
-	 * Reads the content of the input stream.
-	 * If the input stream is empty, the content of the error stream is returned.
-	 *
-	 * @param process the process
-	 * @return the output of the process
-	 * @throws IOException if an error occurs reading stream's bytes
-	 */
-	private static String readProcessOutput(Process process) throws IOException {
-		var inputStream = readStream(process.getInputStream());
-		var output = inputStream.isEmpty() ? readStream(process.getErrorStream()) : inputStream;
-
-		return output.trim();
-	}
-
 	private ProcessHelper() {
 		throw new UnsupportedOperationException();
 	}