From 8d6597a331c9e9031025f15bc1e84e58dfbd3240 Mon Sep 17 00:00:00 2001 From: Roberto Cella <rob.uniud@gmail.com> Date: Wed, 1 Nov 2023 21:37:41 +0100 Subject: [PATCH 1/5] added junit platform to test runtime in order to prepare for gradle 9 --- build.gradle | 1 + gradle/libs.versions.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/build.gradle b/build.gradle index 53512b07..3e2bdacf 100644 --- a/build.gradle +++ b/build.gradle @@ -19,6 +19,7 @@ dependencies { implementation libs.telegram.bot.api implementation libs.tika + testRuntimeOnly libs.junit.platform testImplementation libs.hamcrest testImplementation libs.junit testImplementation libs.mockwebserver diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index cf98a223..d1db67eb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -8,6 +8,7 @@ imageio-webp = "com.twelvemonkeys.imageio:imageio-webp:3.10.0" imgscalr = "org.imgscalr:imgscalr-lib:4.2" jave = "ws.schild:jave-core:3.3.1" junit = "org.junit.jupiter:junit-jupiter:5.10.0" +junit-platform = "org.junit.platform:junit-platform-launcher:1.10.0" logback-classic = { module = "ch.qos.logback:logback-classic", version.ref = "logback" } logback-core = { module = "ch.qos.logback:logback-core", version.ref = "logback" } mockwebserver = "com.squareup.okhttp3:mockwebserver3-junit5:5.0.0-alpha.11" From 4bbde646568dcf296c9e42394e46f3b2dbbaaae7 Mon Sep 17 00:00:00 2001 From: Roberto Cella <rob.uniud@gmail.com> Date: Wed, 1 Nov 2023 21:43:42 +0100 Subject: [PATCH 2/5] improved overall code readability --- .../stickerify/media/MediaHelper.java | 4 +-- .../stickerify/process/PathLocator.java | 3 ++- .../stickerify/process/ProcessHelper.java | 6 ++--- .../stickerify/ResourceHelper.java | 1 - .../stickerify/bot/StickerifyTest.java | 7 +++-- .../junit/TempFilesCleanupExtension.java | 16 ++++++------ .../stickerify/media/MediaHelperTest.java | 26 +++++++++---------- 7 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java index 94341b38..d87b2377 100644 --- a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java @@ -9,6 +9,7 @@ import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_VIDEO_FILE_SIZE; import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_VIDEO_FRAMES; import static com.github.stickerifier.stickerify.media.MediaConstraints.VP9_CODEC; +import static java.nio.charset.StandardCharsets.UTF_8; import com.github.stickerifier.stickerify.process.PathLocator; import com.github.stickerifier.stickerify.process.ProcessHelper; @@ -31,7 +32,6 @@ import java.io.File; import java.io.FileInputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.List; import java.util.zip.GZIPInputStream; @@ -116,7 +116,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th String uncompressedContent = ""; try (var gzipInputStream = new GZIPInputStream(new FileInputStream(file))) { - uncompressedContent = new String(gzipInputStream.readAllBytes(), StandardCharsets.UTF_8); + uncompressedContent = new String(gzipInputStream.readAllBytes(), UTF_8); } catch (IOException e) { LOGGER.atError().log("Unable to retrieve gzip content from file {}", file.getName()); } diff --git a/src/main/java/com/github/stickerifier/stickerify/process/PathLocator.java b/src/main/java/com/github/stickerifier/stickerify/process/PathLocator.java index e20b176a..41c98809 100644 --- a/src/main/java/com/github/stickerifier/stickerify/process/PathLocator.java +++ b/src/main/java/com/github/stickerifier/stickerify/process/PathLocator.java @@ -1,6 +1,7 @@ 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.telegram.exception.TelegramApiException; import org.slf4j.Logger; @@ -23,7 +24,7 @@ public class PathLocator implements ProcessLocator { public PathLocator() { try { if (ffmpegLocation == null || ffmpegLocation.isBlank()) { - ffmpegLocation = ProcessHelper.executeCommand(FIND_FFMPEG).split(System.lineSeparator())[0]; + ffmpegLocation = ProcessHelper.executeCommand(FIND_FFMPEG).split(lineSeparator())[0]; } LOGGER.atInfo().log("FFmpeg is installed at {}", ffmpegLocation); diff --git a/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java b/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java index 133cc2f8..834becb0 100644 --- a/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java @@ -39,11 +39,11 @@ public static String executeCommand(final String[] command) throws TelegramApiEx if (!processExited || process.exitValue() != 0) { var reason = processExited ? "successfully" : "in time"; - var output = toString(process.getErrorStream()); + var output = readStream(process.getErrorStream()); throw new TelegramApiException("The command {} couldn't complete {}: {}", command[0], reason, output); } - return toString(process.getInputStream()); + return readStream(process.getInputStream()); } catch (IOException | InterruptedException e) { throw new TelegramApiException(e); } finally { @@ -54,7 +54,7 @@ public static String executeCommand(final String[] command) throws TelegramApiEx } } - private static String toString(InputStream stream) throws IOException { + private static String readStream(InputStream stream) throws IOException { return new String(stream.readAllBytes(), UTF_8); } diff --git a/src/test/java/com/github/stickerifier/stickerify/ResourceHelper.java b/src/test/java/com/github/stickerifier/stickerify/ResourceHelper.java index 9d641399..9214f7f6 100644 --- a/src/test/java/com/github/stickerifier/stickerify/ResourceHelper.java +++ b/src/test/java/com/github/stickerifier/stickerify/ResourceHelper.java @@ -37,5 +37,4 @@ public File loadResource(String filename) { return new File(resource.getFile()); } - } diff --git a/src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java b/src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java index 5da1b062..9052970e 100644 --- a/src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java +++ b/src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java @@ -1,12 +1,13 @@ package com.github.stickerifier.stickerify.bot; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; import com.github.stickerifier.stickerify.ResourceHelper; -import com.github.stickerifier.stickerify.telegram.Answer; import com.github.stickerifier.stickerify.junit.ClearTempFiles; +import com.github.stickerifier.stickerify.telegram.Answer; import com.pengrad.telegrambot.TelegramBot; import mockwebserver3.MockWebServer; import mockwebserver3.RecordedRequest; @@ -18,7 +19,6 @@ import java.io.File; import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; @ClearTempFiles @ExtendWith(MockWebServerExtension.class) @@ -62,7 +62,7 @@ private void startBot() { } private static void assertResponseContainsMessage(RecordedRequest request, Answer answer) { - var message = URLEncoder.encode(answer.getText(), StandardCharsets.UTF_8); + var message = URLEncoder.encode(answer.getText(), UTF_8); assertThat(request.getBody().readUtf8(), containsString(message)); } @@ -268,5 +268,4 @@ void documentNotSupported() throws Exception { assertEquals("/api/token/sendMessage", sendMessage.getPath()); assertResponseContainsMessage(sendMessage, Answer.ERROR); } - } diff --git a/src/test/java/com/github/stickerifier/stickerify/junit/TempFilesCleanupExtension.java b/src/test/java/com/github/stickerifier/stickerify/junit/TempFilesCleanupExtension.java index e1487143..563f04b0 100644 --- a/src/test/java/com/github/stickerifier/stickerify/junit/TempFilesCleanupExtension.java +++ b/src/test/java/com/github/stickerifier/stickerify/junit/TempFilesCleanupExtension.java @@ -22,21 +22,21 @@ public void afterAll(ExtensionContext context) throws IOException { deleteTempFiles(); } - private static void deleteTempFiles() throws IOException { - try (var files = Files.list(Path.of(System.getProperty("java.io.tmpdir")))) { - files.filter(Files::isRegularFile) - .filter(TempFilesCleanupExtension::stickerifyFiles) - .forEach(TempFilesCleanupExtension::deleteFile); + private void deleteTempFiles() throws IOException { + var tempFolder = System.getProperty("java.io.tmpdir"); + + try (var files = Files.list(Path.of(tempFolder))) { + files.filter(this::stickerifyFiles).forEach(this::deleteFile); } } - private static boolean stickerifyFiles(Path path) { + private boolean stickerifyFiles(Path path) { var fileName = path.getFileName().toString(); - return fileName.startsWith("Stickerify-") || fileName.startsWith("OriginalFile-"); + return Files.isRegularFile(path) && (fileName.startsWith("Stickerify-") || fileName.startsWith("OriginalFile-")); } - private static void deleteFile(Path path) { + private void deleteFile(Path path) { try { Files.delete(path); diff --git a/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java b/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java index 4f27ad42..055e3a07 100644 --- a/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java +++ b/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java @@ -211,12 +211,12 @@ class ConcurrencyTest { @Test @DisplayName("mov videos") void concurrentMovVideoConversions() { - var startingVideo = resources.loadResource("long.mov"); + var movVideo = resources.loadResource("long.mov"); - executeConcurrentConversions(startingVideo); + executeConcurrentConversionsOf(movVideo); } - private static void executeConcurrentConversions(File inputFile) { + private static void executeConcurrentConversionsOf(File inputFile) { final int concurrentRequests = 50; var failedConversions = new AtomicInteger(0); @@ -239,41 +239,41 @@ private static void executeConcurrentConversions(File inputFile) { @Test @DisplayName("mp4 videos") void concurrentMp4VideoConversions() { - var startingVideo = resources.loadResource("video_with_audio.mp4"); + var mp4Video = resources.loadResource("video_with_audio.mp4"); - executeConcurrentConversions(startingVideo); + executeConcurrentConversionsOf(mp4Video); } @Test @DisplayName("webm videos") void concurrentWebmVideoConversions() { - var startingVideo = resources.loadResource("small_video_sticker.webm"); + var webmVideo = resources.loadResource("small_video_sticker.webm"); - executeConcurrentConversions(startingVideo); + executeConcurrentConversionsOf(webmVideo); } @Test @DisplayName("gif videos") void concurrentGifVideoConversions() { - var startingVideo = resources.loadResource("valid.gif"); + var gifVideo = resources.loadResource("valid.gif"); - executeConcurrentConversions(startingVideo); + executeConcurrentConversionsOf(gifVideo); } @Test @DisplayName("webp images") void concurrentWebpImageConversions() { - var startingImage = resources.loadResource("valid.webp"); + var webpImage = resources.loadResource("valid.webp"); - executeConcurrentConversions(startingImage); + executeConcurrentConversionsOf(webpImage); } @Test @DisplayName("png images") void concurrentPngImageConversions() { - var startingImage = resources.createImage(256, 256, "png"); + var pngImage = resources.createImage(256, 256, "png"); - executeConcurrentConversions(startingImage); + executeConcurrentConversionsOf(pngImage); } } } From 3f8e0a54c0433d4cd15c98f9a728a91159e87128 Mon Sep 17 00:00:00 2001 From: Roberto Cella <rob.uniud@gmail.com> Date: Wed, 1 Nov 2023 21:54:41 +0100 Subject: [PATCH 3/5] renamed media variables in unit test --- .../stickerify/media/MediaHelperTest.java | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java b/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java index 055e3a07..4046a998 100644 --- a/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java +++ b/src/test/java/com/github/stickerifier/stickerify/media/MediaHelperTest.java @@ -48,8 +48,8 @@ void setup() { @Test void resizeImage() throws Exception { - var startingImage = resources.createImage(1024, 1024, "jpg"); - var result = MediaHelper.convert(startingImage); + var jpgImage = resources.createImage(1024, 1024, "jpg"); + var result = MediaHelper.convert(jpgImage); assertImageConsistency(result, 512, 512); } @@ -67,40 +67,40 @@ private static void assertImageConsistency(File result, int expectedWidth, int e @Test void resizeRectangularImage() throws Exception { - var startingImage = resources.createImage(1024, 512, "jpg"); - var result = MediaHelper.convert(startingImage); + var jpgImage = resources.createImage(1024, 512, "jpg"); + var result = MediaHelper.convert(jpgImage); assertImageConsistency(result, 512, 256); } @Test void resizeSmallImage() throws Exception { - var startingImage = resources.createImage(256, 256, "png"); - var result = MediaHelper.convert(startingImage); + var pngImage = resources.createImage(256, 256, "png"); + var result = MediaHelper.convert(pngImage); assertImageConsistency(result, 512, 512); } @Test void noImageConversionNeeded() throws Exception { - var startingImage = resources.createImage(512, 256, "png"); - var result = MediaHelper.convert(startingImage); + var pngImage = resources.createImage(512, 256, "png"); + var result = MediaHelper.convert(pngImage); assertThat(result, is(nullValue())); } @Test void resizeWebpImage() throws Exception { - var startingImage = resources.loadResource("valid.webp"); - var result = MediaHelper.convert(startingImage); + var webpImage = resources.loadResource("valid.webp"); + var result = MediaHelper.convert(webpImage); assertImageConsistency(result, 256, 512); } @Test void convertLongMovVideo() throws Exception { - var startingVideo = resources.loadResource("long.mov"); - var result = MediaHelper.convert(startingVideo); + var movVideo = resources.loadResource("long.mov"); + var result = MediaHelper.convert(movVideo); assertVideoConsistency(result, 512, 288, 29.97F, 3_000L); } @@ -127,48 +127,48 @@ private static void assertVideoConsistency(File result, int expectedWidth, int e @Test void convertMp4WithAudio() throws Exception { - var startingVideo = resources.loadResource("video_with_audio.mp4"); - var result = MediaHelper.convert(startingVideo); + var mp4Video = resources.loadResource("video_with_audio.mp4"); + var result = MediaHelper.convert(mp4Video); assertVideoConsistency(result, 512, 288, 29.97F, 3_000L); } @Test void convertShortAndLowFpsVideo() throws Exception { - var startingVideo = resources.loadResource("short_low_fps.webm"); - var result = MediaHelper.convert(startingVideo); + var webmVideo = resources.loadResource("short_low_fps.webm"); + var result = MediaHelper.convert(webmVideo); assertVideoConsistency(result, 512, 288, 10F, 1_000L); } @Test void resizeSmallWebmVideo() throws Exception { - var startingVideo = resources.loadResource("small_video_sticker.webm"); - var result = MediaHelper.convert(startingVideo); + var webmVideo = resources.loadResource("small_video_sticker.webm"); + var result = MediaHelper.convert(webmVideo); assertVideoConsistency(result, 512, 212, 30F, 2_000L); } @Test void convertVerticalWebmVideo() throws Exception { - var startingVideo = resources.loadResource("vertical_video_sticker.webm"); - var result = MediaHelper.convert(startingVideo); + var webmVideo = resources.loadResource("vertical_video_sticker.webm"); + var result = MediaHelper.convert(webmVideo); assertVideoConsistency(result, 288, 512, 30F, 2_000L); } @Test void convertGifVideo() throws Exception { - var startingVideo = resources.loadResource("valid.gif"); - var result = MediaHelper.convert(startingVideo); + var gifVideo = resources.loadResource("valid.gif"); + var result = MediaHelper.convert(gifVideo); assertVideoConsistency(result, 512, 274, 10F, 1_000L); } @Test void noVideoConversionNeeded() throws Exception { - var startingVideo = resources.loadResource("no_conversion_needed.webm"); - var result = MediaHelper.convert(startingVideo); + var webmVideo = resources.loadResource("no_conversion_needed.webm"); + var result = MediaHelper.convert(webmVideo); assertThat(result, is(nullValue())); } From 3c695dbd33bed669ea12213c553337c2691b367b Mon Sep 17 00:00:00 2001 From: Roberto Cella <rob.uniud@gmail.com> Date: Wed, 1 Nov 2023 22:12:47 +0100 Subject: [PATCH 4/5] added javadoc to readStream method and used it to avoid code duplication in MediaHelper --- .../stickerifier/stickerify/media/MediaHelper.java | 5 ++--- .../stickerifier/stickerify/process/ProcessHelper.java | 9 ++++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java index d87b2377..bc3d780c 100644 --- a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java @@ -9,7 +9,6 @@ import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_VIDEO_FILE_SIZE; import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_VIDEO_FRAMES; import static com.github.stickerifier.stickerify.media.MediaConstraints.VP9_CODEC; -import static java.nio.charset.StandardCharsets.UTF_8; import com.github.stickerifier.stickerify.process.PathLocator; import com.github.stickerifier.stickerify.process.ProcessHelper; @@ -116,7 +115,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th String uncompressedContent = ""; try (var gzipInputStream = new GZIPInputStream(new FileInputStream(file))) { - uncompressedContent = new String(gzipInputStream.readAllBytes(), UTF_8); + uncompressedContent = ProcessHelper.readStream(gzipInputStream); } catch (IOException e) { LOGGER.atError().log("Unable to retrieve gzip content from file {}", file.getName()); } @@ -135,7 +134,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th private record AnimationDetails(@SerializedName("w") int width, @SerializedName("h") int height, @SerializedName("fr") int frameRate, @SerializedName("ip") float start, @SerializedName("op") float end) { private float duration() { - return (end() - start()) / frameRate(); + return (end - start) / frameRate; } } diff --git a/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java b/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java index 834becb0..c0b9b04a 100644 --- a/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java @@ -54,7 +54,14 @@ public static String executeCommand(final String[] command) throws TelegramApiEx } } - private static String readStream(InputStream stream) throws IOException { + /** + * 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 + */ + public static String readStream(InputStream stream) throws IOException { return new String(stream.readAllBytes(), UTF_8); } From 1f51682a8b22499667f41fe48d3f553b52502248 Mon Sep 17 00:00:00 2001 From: Roberto Cella <rob.uniud@gmail.com> Date: Mon, 6 Nov 2023 13:46:33 +0100 Subject: [PATCH 5/5] made readStream method private to avoid misuses of process helper class --- .../com/github/stickerifier/stickerify/media/MediaHelper.java | 3 ++- .../github/stickerifier/stickerify/process/ProcessHelper.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java index bc3d780c..0a514bc4 100644 --- a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java @@ -9,6 +9,7 @@ import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_VIDEO_FILE_SIZE; import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_VIDEO_FRAMES; import static com.github.stickerifier.stickerify.media.MediaConstraints.VP9_CODEC; +import static java.nio.charset.StandardCharsets.UTF_8; import com.github.stickerifier.stickerify.process.PathLocator; import com.github.stickerifier.stickerify.process.ProcessHelper; @@ -115,7 +116,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th String uncompressedContent = ""; try (var gzipInputStream = new GZIPInputStream(new FileInputStream(file))) { - uncompressedContent = ProcessHelper.readStream(gzipInputStream); + uncompressedContent = new String(gzipInputStream.readAllBytes(), UTF_8); } catch (IOException e) { LOGGER.atError().log("Unable to retrieve gzip content from file {}", file.getName()); } diff --git a/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java b/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java index c0b9b04a..90a0e399 100644 --- a/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/process/ProcessHelper.java @@ -61,7 +61,7 @@ public static String executeCommand(final String[] command) throws TelegramApiEx * @return the UTF-8 representation of passed-in stream * @throws IOException if an error occurs reading stream's bytes */ - public static String readStream(InputStream stream) throws IOException { + private static String readStream(InputStream stream) throws IOException { return new String(stream.readAllBytes(), UTF_8); }