diff --git a/src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java b/src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java index 2aa16d58..6a1a4387 100644 --- a/src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java +++ b/src/main/java/com/github/stickerifier/stickerify/bot/Stickerify.java @@ -1,7 +1,10 @@ package com.github.stickerifier.stickerify.bot; -import static com.github.stickerifier.stickerify.logger.StructuredLogger.REQUEST_DETAILS; -import static com.github.stickerifier.stickerify.telegram.Answer.CORRUPTED; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.EXCEPTION_MESSAGE_LOG_KEY; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.FILE_ID_VALUE; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.FILE_PATH_LOG_KEY; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.ORIGINAL_REQUEST_LOG_KEY; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.REQUEST_DETAILS_VALUE; import static com.github.stickerifier.stickerify.telegram.Answer.ERROR; import static com.github.stickerifier.stickerify.telegram.Answer.FILE_ALREADY_VALID; import static com.github.stickerifier.stickerify.telegram.Answer.FILE_READY; @@ -10,7 +13,6 @@ import static java.util.HashSet.newHashSet; import static java.util.concurrent.Executors.newThreadPerTaskExecutor; -import com.github.stickerifier.stickerify.exception.CorruptedFileException; import com.github.stickerifier.stickerify.exception.FileOperationException; import com.github.stickerifier.stickerify.exception.TelegramApiException; import com.github.stickerifier.stickerify.logger.StructuredLogger; @@ -77,7 +79,7 @@ public int process(List updates) { updates.forEach(update -> executor.execute(() -> { if (update.message() != null) { var request = new TelegramRequest(update.message()); - ScopedValue.where(REQUEST_DETAILS, request.toRequestDetails()).run(() -> answer(request)); + ScopedValue.where(REQUEST_DETAILS_VALUE, request.toRequestDetails()).run(() -> answer(request)); } })); @@ -86,7 +88,10 @@ public int process(List updates) { @Override public void onException(TelegramException e) { - LOGGER.at(Level.ERROR).setCause(e).addKeyValue("exception_message", e.getMessage()).log("An unexpected failure occurred"); + LOGGER.at(Level.ERROR) + .setCause(e) + .addKeyValue(EXCEPTION_MESSAGE_LOG_KEY, e.getMessage()) + .log("An unexpected failure occurred"); } @Override @@ -116,7 +121,7 @@ private void answerFile(TelegramRequest request, TelegramFile file) { if (file == TelegramFile.NOT_SUPPORTED) { answerText(ERROR, request); } else if (file.canBeDownloaded()) { - answerFile(request, file.id()); + ScopedValue.where(FILE_ID_VALUE, file.id()).run(() -> answerFile(request, file.id())); } else { LOGGER.at(Level.INFO).log("Passed-in file is too large"); @@ -149,7 +154,7 @@ private void answerFile(TelegramRequest request, String fileId) { } catch (InterruptedException e) { Thread.currentThread().interrupt(); } catch (Exception e) { - processFailure(request, e, fileId); + processFailure(request, e); } finally { deleteTempFiles(pathsToDelete); } @@ -169,7 +174,7 @@ private File retrieveFile(String fileId) throws TelegramApiException, FileOperat } } - private void processFailure(TelegramRequest request, Exception e, String fileId) { + private void processFailure(TelegramRequest request, Exception e) { if (e instanceof TelegramApiException telegramException) { boolean replyToUser = processTelegramFailure(telegramException, false); if (!replyToUser) { @@ -177,13 +182,8 @@ private void processFailure(TelegramRequest request, Exception e, String fileId) } } - if (e instanceof CorruptedFileException) { - LOGGER.at(Level.INFO).log("Unable to reply to the request: the file is corrupted"); - answerText(CORRUPTED, request); - } else { - LOGGER.at(Level.WARN).setCause(e).addKeyValue("file_id", fileId).log("Unable to process file"); - answerText(ERROR, request); - } + LOGGER.at(Level.ERROR).setCause(e).log("Unable to process file"); + answerText(ERROR, request); } private boolean processTelegramFailure(TelegramApiException e, boolean logUnmatchedFailure) { @@ -206,7 +206,7 @@ private boolean processTelegramFailure(TelegramApiException e, boolean logUnmatc private void answerText(TelegramRequest request) { var message = request.message(); if (message.text() == null) { - LOGGER.at(Level.INFO).log("An unhandled message type has been received"); + LOGGER.at(Level.INFO).addKeyValue(ORIGINAL_REQUEST_LOG_KEY, message).log("An unhandled message type has been received"); } answerText(request.getAnswerMessage(), request); @@ -241,10 +241,10 @@ private static void deleteTempFiles(Set pathsToDelete) { for (var path : pathsToDelete) { try { if (!Files.deleteIfExists(path)) { - LOGGER.at(Level.INFO).addKeyValue("file_path", path).log("Unable to delete temp file"); + LOGGER.at(Level.INFO).addKeyValue(FILE_PATH_LOG_KEY, path).log("Unable to delete temp file"); } } catch (IOException e) { - LOGGER.at(Level.ERROR).setCause(e).addKeyValue("file_path", path).log("An error occurred trying to delete temp file"); + LOGGER.at(Level.ERROR).setCause(e).addKeyValue(FILE_PATH_LOG_KEY, path).log("An error occurred trying to delete temp file"); } } } diff --git a/src/main/java/com/github/stickerifier/stickerify/exception/CorruptedFileException.java b/src/main/java/com/github/stickerifier/stickerify/exception/CorruptedFileException.java deleted file mode 100644 index 6c04efc9..00000000 --- a/src/main/java/com/github/stickerifier/stickerify/exception/CorruptedFileException.java +++ /dev/null @@ -1,7 +0,0 @@ -package com.github.stickerifier.stickerify.exception; - -public class CorruptedFileException extends MediaException { - public CorruptedFileException(String message, Throwable cause) { - super(message, cause); - } -} diff --git a/src/main/java/com/github/stickerifier/stickerify/logger/StructuredLogger.java b/src/main/java/com/github/stickerifier/stickerify/logger/StructuredLogger.java index 517a0106..3bef1bc1 100644 --- a/src/main/java/com/github/stickerifier/stickerify/logger/StructuredLogger.java +++ b/src/main/java/com/github/stickerifier/stickerify/logger/StructuredLogger.java @@ -8,15 +8,25 @@ public record StructuredLogger(Logger logger) { - public static final ScopedValue REQUEST_DETAILS = ScopedValue.newInstance(); - public static final ScopedValue MIME_TYPE = ScopedValue.newInstance(); + public static final ScopedValue REQUEST_DETAILS_VALUE = ScopedValue.newInstance(); + public static final ScopedValue FILE_ID_VALUE = ScopedValue.newInstance(); + public static final ScopedValue MIME_TYPE_VALUE = ScopedValue.newInstance(); + + private static final String REQUEST_DETAILS_LOG_KEY = "request_details"; + private static final String FILE_ID_LOG_KEY = "file_id"; + private static final String MIME_TYPE_LOG_KEY = "mime_type"; + public static final String EXCEPTION_MESSAGE_LOG_KEY = "exception_message"; + public static final String ORIGINAL_REQUEST_LOG_KEY = "original_request"; + public static final String FILE_PATH_LOG_KEY = "file_path"; + public static final String STICKER_LOG_KEY = "sticker"; public StructuredLogger(Class clazz) { this(LoggerFactory.getLogger(clazz)); } /** - * Creates a {@link LoggingEventBuilder} at the specified level with request details and MIME type information, if set. + * Creates a {@link LoggingEventBuilder} at the specified level enriched with request details, + * file id, and MIME type information when available. * * @param level the level of the log * @return the log builder with context information @@ -24,11 +34,14 @@ public StructuredLogger(Class clazz) { public LoggingEventBuilder at(Level level) { var logBuilder = logger.atLevel(level); - if (REQUEST_DETAILS.isBound()) { - logBuilder = logBuilder.addKeyValue("request_details", REQUEST_DETAILS.get()); + if (REQUEST_DETAILS_VALUE.isBound()) { + logBuilder = logBuilder.addKeyValue(REQUEST_DETAILS_LOG_KEY, REQUEST_DETAILS_VALUE.get()); + } + if (FILE_ID_VALUE.isBound()) { + logBuilder = logBuilder.addKeyValue(FILE_ID_LOG_KEY, FILE_ID_VALUE.get()); } - if (MIME_TYPE.isBound()) { - logBuilder = logBuilder.addKeyValue("mime_type", MIME_TYPE.get()); + if (MIME_TYPE_VALUE.isBound()) { + logBuilder = logBuilder.addKeyValue(MIME_TYPE_LOG_KEY, MIME_TYPE_VALUE.get()); } return logBuilder; 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 81d4c225..9932f8f0 100644 --- a/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java +++ b/src/main/java/com/github/stickerifier/stickerify/media/MediaHelper.java @@ -1,6 +1,8 @@ package com.github.stickerifier.stickerify.media; -import static com.github.stickerifier.stickerify.logger.StructuredLogger.MIME_TYPE; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.FILE_PATH_LOG_KEY; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.MIME_TYPE_VALUE; +import static com.github.stickerifier.stickerify.logger.StructuredLogger.STICKER_LOG_KEY; import static com.github.stickerifier.stickerify.media.MediaConstraints.MATROSKA_FORMAT; import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_ANIMATION_DURATION_SECONDS; import static com.github.stickerifier.stickerify.media.MediaConstraints.MAX_ANIMATION_FILE_SIZE; @@ -14,7 +16,6 @@ import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.nio.charset.StandardCharsets.UTF_8; -import com.github.stickerifier.stickerify.exception.CorruptedFileException; import com.github.stickerifier.stickerify.exception.FileOperationException; import com.github.stickerifier.stickerify.exception.MediaException; import com.github.stickerifier.stickerify.exception.ProcessException; @@ -67,7 +68,7 @@ public final class MediaHelper { public static @Nullable File convert(File inputFile) throws Exception { var mimeType = detectMimeType(inputFile); - return ScopedValue.where(MIME_TYPE, mimeType).call(() -> performConversion(inputFile, mimeType)); + return ScopedValue.where(MIME_TYPE_VALUE, mimeType).call(() -> performConversion(inputFile, mimeType)); } /** @@ -81,7 +82,7 @@ private static String detectMimeType(File file) throws MediaException { try { return TIKA.detect(file); } catch (IOException e) { - LOGGER.at(Level.ERROR).setCause(e).addKeyValue("file_name", file.getName()).log("Unable to retrieve MIME type"); + LOGGER.at(Level.ERROR).setCause(e).addKeyValue(FILE_PATH_LOG_KEY, file.getPath()).log("Unable to retrieve MIME type"); throw new MediaException(e); } } @@ -144,10 +145,10 @@ private static boolean isSupportedVideo(String mimeType) { * * @param file the file to check * @return {@code true} if the file is compliant - * @throws FileOperationException if an error occurred retrieving the size of the file + * @throws MediaException if an error occurred retrieving video information * @throws InterruptedException if the current thread is interrupted while retrieving file info */ - private static boolean isVideoCompliant(File file) throws FileOperationException, CorruptedFileException, InterruptedException { + private static boolean isVideoCompliant(File file) throws MediaException, InterruptedException { var mediaInfo = retrieveMultimediaInfo(file); var formatInfo = mediaInfo.format(); @@ -178,10 +179,10 @@ private static boolean isVideoCompliant(File file) throws FileOperationException * * @param file the video to check * @return passed-in video's multimedia information - * @throws CorruptedFileException if an error occurred retrieving file information + * @throws MediaException if an error occurred retrieving file information * @throws InterruptedException if the current thread is interrupted while retrieving file info */ - static MultimediaInfo retrieveMultimediaInfo(File file) throws CorruptedFileException, InterruptedException { + static MultimediaInfo retrieveMultimediaInfo(File file) throws MediaException, InterruptedException { var command = new String[] { "ffprobe", "-hide_banner", @@ -197,7 +198,7 @@ static MultimediaInfo retrieveMultimediaInfo(File file) throws CorruptedFileExce return GSON.fromJson(output, MultimediaInfo.class); } catch (ProcessException | JsonSyntaxException e) { - throw new CorruptedFileException("The file could not be processed successfully", e); + throw new MediaException("Unable to retrieve media information", e); } } @@ -253,7 +254,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th try (var gzipInputStream = new GZIPInputStream(new FileInputStream(file))) { uncompressedContent = new String(gzipInputStream.readAllBytes(), UTF_8); } catch (IOException e) { - LOGGER.at(Level.ERROR).setCause(e).addKeyValue("file_name", file.getName()).log("Unable to retrieve gzip content"); + LOGGER.at(Level.ERROR).setCause(e).addKeyValue(FILE_PATH_LOG_KEY, file.getPath()).log("Unable to retrieve gzip content"); } try { @@ -267,7 +268,7 @@ private static boolean isAnimatedStickerCompliant(File file, String mimeType) th } } - LOGGER.at(Level.WARN).addKeyValue("sticker", sticker).log("The animated sticker doesn't meet Telegram's requirements"); + LOGGER.at(Level.WARN).addKeyValue(STICKER_LOG_KEY, sticker).log("The animated sticker doesn't meet Telegram's requirements"); } catch (JsonSyntaxException _) { LOGGER.at(Level.INFO).log("The archive isn't an animated sticker"); } @@ -354,10 +355,10 @@ private static boolean isAnimatedWebp(File file) { * @param image the image to check * @param mimeType the MIME type of the file * @return {@code true} if the file is compliant - * @throws CorruptedFileException if an error occurred retrieving image information + * @throws MediaException if an error occurred retrieving image information * @throws InterruptedException if the current thread is interrupted while retrieving file info */ - private static boolean isImageCompliant(File image, String mimeType) throws CorruptedFileException, InterruptedException { + private static boolean isImageCompliant(File image, String mimeType) throws MediaException, InterruptedException { var mediaInfo = retrieveMultimediaInfo(image); var formatInfo = mediaInfo.format(); @@ -448,7 +449,7 @@ private static File createTempFile(String fileExtension) throws FileOperationExc private static void deleteFile(File file) throws FileOperationException { try { if (!Files.deleteIfExists(file.toPath())) { - LOGGER.at(Level.INFO).addKeyValue("file_path", file.toPath()).log("Unable to delete file"); + LOGGER.at(Level.INFO).addKeyValue(FILE_PATH_LOG_KEY, file.toPath()).log("Unable to delete file"); } } catch (IOException e) { throw new FileOperationException("An error occurred deleting the file", e); @@ -493,11 +494,10 @@ private static File convertToWebm(File file) throws MediaException, InterruptedE } throw new MediaException("FFmpeg two-pass conversion failed", e); } finally { - var logFileName = logPrefix + "-0.log"; try { - deleteFile(new File(logFileName)); + deleteFile(new File(logPrefix + "-0.log")); } catch (FileOperationException e) { - LOGGER.at(Level.WARN).setCause(e).addKeyValue("file_name", logFileName).log("Could not delete log file"); + LOGGER.at(Level.WARN).setCause(e).log("Could not delete log file"); } } diff --git a/src/main/java/com/github/stickerifier/stickerify/telegram/Answer.java b/src/main/java/com/github/stickerifier/stickerify/telegram/Answer.java index 36eac391..a58ce5c2 100644 --- a/src/main/java/com/github/stickerifier/stickerify/telegram/Answer.java +++ b/src/main/java/com/github/stickerifier/stickerify/telegram/Answer.java @@ -35,11 +35,6 @@ public enum Answer { ERROR(""" The file conversion was unsuccessful: only images, gifs, standard and video stickers are supported\\. - If you think it should have worked, please report the issue on [Github](https://github.com/Stickerifier/Stickerify/issues/new/choose)\\. - """, true), - CORRUPTED(""" - The conversion was unsuccessful: the video might be corrupted and it cannot be processed\\. - If you think it should have worked, please report the issue on [Github](https://github.com/Stickerifier/Stickerify/issues/new/choose)\\. """, true), PRIVACY_POLICY(""" diff --git a/src/main/resources/logback.xml b/src/main/resources/logback.xml index 68b1f0c1..027e1531 100644 --- a/src/main/resources/logback.xml +++ b/src/main/resources/logback.xml @@ -1,12 +1,9 @@ - + - - UTC - @@ -24,7 +21,7 @@ - + diff --git a/src/test/java/com/github/stickerifier/stickerify/bot/MockResponses.java b/src/test/java/com/github/stickerifier/stickerify/bot/MockResponses.java index a047ee7f..88d6d4d3 100644 --- a/src/test/java/com/github/stickerifier/stickerify/bot/MockResponses.java +++ b/src/test/java/com/github/stickerifier/stickerify/bot/MockResponses.java @@ -295,30 +295,6 @@ public final class MockResponses { } """).build(); - static final MockResponse CORRUPTED_FILE = new MockResponse.Builder().body(""" - { - ok: true, - result: [ - { - update_id: 1, - message: { - message_id: 1, - from: { - id: 123456 - }, - chat: { - id: 1 - }, - video: { - file_id: "corrupted.mp4", - file_size: 200000 - } - } - } - ] - } - """).build(); - static MockResponse fileInfo(String fileName) { return new MockResponse.Builder().body(""" { 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 9e1f31c8..0a58d1f4 100644 --- a/src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java +++ b/src/test/java/com/github/stickerifier/stickerify/bot/StickerifyTest.java @@ -290,28 +290,4 @@ void documentNotSupported() throws Exception { assertResponseContainsMessage(sendMessage, Answer.ERROR); } } - - @Test - void corruptedVideo() throws Exception { - server.enqueue(MockResponses.CORRUPTED_FILE); - server.enqueue(MockResponses.fileInfo("corrupted.mp4")); - server.enqueue(MockResponses.fileDownload("corrupted.mp4")); - - try (var _ = runBot()) { - var getUpdates = server.takeRequest(); - assertEquals("/api/token/getUpdates", getUpdates.getTarget()); - - var getFile = server.takeRequest(); - assertEquals("/api/token/getFile", getFile.getTarget()); - assertNotNull(getFile.getBody()); - assertEquals("file_id=corrupted.mp4", getFile.getBody().utf8()); - - var download = server.takeRequest(); - assertEquals("/files/token/corrupted.mp4", download.getTarget()); - - var sendMessage = server.takeRequest(); - assertEquals("/api/token/sendMessage", sendMessage.getTarget()); - assertResponseContainsMessage(sendMessage, Answer.CORRUPTED); - } - } } diff --git a/src/test/resources/corrupted.mp4 b/src/test/resources/corrupted.mp4 deleted file mode 100644 index c77b4b20..00000000 Binary files a/src/test/resources/corrupted.mp4 and /dev/null differ