Code review comments

Author: MartelliEnrico

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

@@ -147,10 +147,10 @@ private static boolean isVideoCompliant(File file) throws FileOperationException
 		return isSizeCompliant(videoInfo.width(), videoInfo.height())
 				&& videoInfo.frameRate() <= MAX_VIDEO_FRAMES
 				&& VP9_CODEC.equals(videoInfo.codec())
-				&& Float.parseFloat(formatInfo.duration()) <= MAX_VIDEO_DURATION_SECONDS
+				&& formatInfo.duration() <= MAX_VIDEO_DURATION_SECONDS
 				&& mediaInfo.audio() == null
 				&& formatInfo.format().startsWith(MATROSKA_FORMAT)
-				&& Long.parseLong(formatInfo.size()) <= MAX_VIDEO_FILE_SIZE;
+				&& formatInfo.size() <= MAX_VIDEO_FILE_SIZE;
 	}
 
 	/**
@@ -173,8 +173,7 @@ static MultimediaInfo retrieveMultimediaInfo(File file) throws CorruptedFileExce
 		};
 
 		try {
-			var lines = ProcessHelper.executeCommand(command);
-			var body = String.join("\n", lines);
+			var body = ProcessHelper.executeCommand(command);
 
 			return GSON.fromJson(body, MultimediaInfo.class);
 		} catch (ProcessException | JsonSyntaxException e) {
@@ -183,20 +182,23 @@ static MultimediaInfo retrieveMultimediaInfo(File file) throws CorruptedFileExce
 	}
 
 	record MultimediaInfo(List<StreamInfo> streams, @Nullable FormatInfo format) {
-		@Nullable StreamInfo audio() {
+		@Nullable
+		StreamInfo audio() {
 			return streams.stream()
 					.filter(s -> s.type == CodecType.AUDIO)
 					.findFirst()
 					.orElse(null);
 		}
 
-		@Nullable StreamInfo video() {
+		@Nullable
+		StreamInfo video() {
 			return streams.stream()
 					.filter(s -> s.type == CodecType.VIDEO)
 					.findFirst()
 					.orElse(null);
 		}
 	}
+
 	record StreamInfo(@SerializedName("codec_name") String codec, @SerializedName("codec_type") CodecType type, int width, int height, @SerializedName("avg_frame_rate") String frameRateRatio) {
 		float frameRate() {
 			if (frameRateRatio.contains("/")) {
@@ -207,11 +209,13 @@ float frameRate() {
 			}
 		}
 	}
+
 	private enum CodecType {
 		@SerializedName("video") VIDEO,
 		@SerializedName("audio") AUDIO
 	}
-	record FormatInfo(@SerializedName("format_name") String format, @Nullable String duration, String size) {}
+
+	record FormatInfo(@SerializedName("format_name") String format, @Nullable Float duration, long size) {}
 
 	/**
 	 * Checks if the file is a {@code gzip} archive, then it reads its content and verifies if it's a valid JSON.
@@ -301,14 +305,14 @@ private static boolean isImageCompliant(File image, String mimeType) throws Corr
 			return false;
 		}
 
-		var videoInfo = mediaInfo.video();
-		if (videoInfo == null) {
+		var imageInfo = mediaInfo.video();
+		if (imageInfo == null) {
 			return false;
 		}
 
 		return ("image/png".equals(mimeType) || "image/webp".equals(mimeType))
-				&& isSizeCompliant(videoInfo.width(), videoInfo.height())
-				&& Long.parseLong(formatInfo.size()) <= MAX_IMAGE_FILE_SIZE;
+				&& isSizeCompliant(imageInfo.width(), imageInfo.height())
+				&& formatInfo.size() <= MAX_IMAGE_FILE_SIZE;
 	}
 
 	/**

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

@@ -7,8 +7,7 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.StringJoiner;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
 
@@ -23,7 +22,7 @@ public final class ProcessHelper {
 	 * environment variable (defaults to 4).
 	 *
 	 * @param command the command to be executed
-	 * @return the merged stdout/stderr of the command, split by lines
+	 * @return the merged stdout/stderr of the command
 	 * @throws ProcessException either if:
 	 * <ul>
 	 *     <li>the command was unsuccessful
@@ -33,12 +32,12 @@ public final class ProcessHelper {
 	 * </ul>
 	 * @throws InterruptedException if the current thread is interrupted while waiting for the command to finish
 	 */
-	public static List<String> executeCommand(final String... command) throws ProcessException, InterruptedException {
+	public static String executeCommand(final String... command) throws ProcessException, InterruptedException {
 		SEMAPHORE.acquire();
 		try {
 			var process = new ProcessBuilder(command).redirectErrorStream(true).start();
 
-			var output = new ArrayList<String>(64);
+			var output = new StringJoiner("\n");
 			var readerThread = Thread.ofVirtual().start(() -> {
 				try (var reader = process.inputReader(UTF_8)) {
 					reader.lines().forEach(output::add);
@@ -51,17 +50,16 @@ public static List<String> executeCommand(final String... command) throws Proces
 			if (!finished) {
 				process.destroyForcibly();
 				readerThread.join();
-				throw new ProcessException("The command {} timed out after 1m\n{}", command[0], String.join("\n", output));
+				throw new ProcessException("The command {} timed out after 1m\n{}", command[0], output.toString());
 			}
 
 			readerThread.join();
 			var exitCode = process.exitValue();
 			if (exitCode != 0) {
-				var lines = String.join("\n", output);
-				throw new ProcessException("The command {} exited with code {}\n{}", command[0], exitCode, lines);
+				throw new ProcessException("The command {} exited with code {}\n{}", command[0], exitCode, output.toString());
 			}
 
-			return output;
+			return output.toString();
 		} catch (IOException e) {
 			throw new ProcessException(e);
 		} finally {

src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java

@@ -28,7 +28,6 @@
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 
 import java.io.File;
-import java.nio.file.Files;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.stream.IntStream;
@@ -47,15 +46,18 @@ void resizeRectangularImage() throws Exception {
 	}
 
 	private static void assertImageConsistency(File image, int expectedWidth, int expectedHeight) throws Exception {
-		var imageInfo = MediaHelper.retrieveMultimediaInfo(image).video();
+		var mediaInfo = MediaHelper.retrieveMultimediaInfo(image);
+		var imageInfo = mediaInfo.video();
 		assertNotNull(imageInfo);
+		var formatInfo = mediaInfo.format();
+		assertNotNull(formatInfo);
 		var actualExtension = getExtension(image);
 
 		assertAll("Image validation failed",
 				() -> assertThat("image's extension must be webp", actualExtension, is(equalTo(".webp"))),
 				() -> assertThat("image's width is not correct", imageInfo.width(), is(equalTo(expectedWidth))),
 				() -> assertThat("image's height is not correct", imageInfo.height(), is(equalTo(expectedHeight))),
-				() -> assertThat("image size should not exceed 512 KB", Files.size(image.toPath()), is(lessThanOrEqualTo(MAX_IMAGE_FILE_SIZE)))
+				() -> assertThat("image size should not exceed 512 KB", formatInfo.size(), is(lessThanOrEqualTo(MAX_IMAGE_FILE_SIZE)))
 		);
 	}
 
@@ -138,8 +140,8 @@ void resizeSvgImage() throws Exception {
 	}
 
 	private static void assumeSvgSupport() throws Exception {
-		var lines = ProcessHelper.executeCommand("ffmpeg", "-v", "quiet", "-hide_banner", "-decoders");
-		var supportsSvg = lines.stream().anyMatch(line -> line.contains("(codec svg)"));
+		var decoders = ProcessHelper.executeCommand("ffmpeg", "-v", "quiet", "-hide_banner", "-decoders");
+		var supportsSvg = decoders.contains("(codec svg)");
 		assumeTrue(supportsSvg, "FFmpeg was not compiled with SVG support");
 	}
 
@@ -168,10 +170,10 @@ private static void assertVideoConsistency(File video, int expectedWidth, int ex
 				() -> assertThat("video's height is not correct", videoInfo.height(), is(equalTo(expectedHeight))),
 				() -> assertThat("video's frame rate is not correct", videoInfo.frameRate(), is(equalTo(expectedFrameRate))),
 				() -> assertThat("video must be encoded with the VP9 codec", videoInfo.codec(), is(equalTo(VP9_CODEC))),
-				() -> assertThat("video's duration is not correct", Float.parseFloat(formatInfo.duration()), is(equalTo(expectedDuration))),
+				() -> assertThat("video's duration is not correct", formatInfo.duration(), is(equalTo(expectedDuration))),
 				() -> assertThat("video's format must be matroska", formatInfo.format(), startsWith(MATROSKA_FORMAT)),
 				() -> assertThat("video must have no audio stream", mediaInfo.audio(), is(nullValue())),
-				() -> assertThat("video size should not exceed 256 KB", Files.size(video.toPath()), is(lessThanOrEqualTo(MAX_VIDEO_FILE_SIZE)))
+				() -> assertThat("video size should not exceed 256 KB", formatInfo.size(), is(lessThanOrEqualTo(MAX_VIDEO_FILE_SIZE)))
 		);
 	}