From 9e41c625a1c9a008685972c5b9ec781f9389781f Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Mon, 14 Jul 2025 11:32:28 +0100 Subject: [PATCH] AOP Fixes for v2 async (#3934) # Description of Changes --- ## Checklist ### General - [ ] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [ ] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [ ] I have performed a self-review of my own code - [ ] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --- .../software/common/aop/AutoJobAspect.java | 89 ++----------------- .../software/common/model/job/JobResult.java | 3 + .../software/common/service/TaskManager.java | 6 +- .../SPDF/config/CleanUrlInterceptor.java | 3 +- 4 files changed, 14 insertions(+), 87 deletions(-) diff --git a/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java b/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java index 51c1882b6..9f01c4558 100644 --- a/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java +++ b/common/src/main/java/stirling/software/common/aop/AutoJobAspect.java @@ -43,6 +43,7 @@ public class AutoJobAspect { // This aspect will run before any audit aspects due to @Order(0) // Extract parameters from the request and annotation boolean async = Boolean.parseBoolean(request.getParameter("async")); + log.debug("AutoJobAspect: Processing {} {} with async={}", request.getMethod(), request.getRequestURI(), async); long timeout = autoJobPostMapping.timeout(); int retryCount = autoJobPostMapping.retryCount(); boolean trackProgress = autoJobPostMapping.trackProgress(); @@ -54,19 +55,8 @@ public class AutoJobAspect { retryCount, trackProgress); - // Copy and process arguments - // In a test environment, we might need to update the original objects for verification - boolean isTestEnvironment = false; - try { - isTestEnvironment = Class.forName("org.junit.jupiter.api.Test") != null; - } catch (ClassNotFoundException e) { - // Not in a test environment - } - - Object[] args = - isTestEnvironment - ? processArgsInPlace(joinPoint.getArgs(), async) - : copyAndProcessArgs(joinPoint.getArgs(), async); + // Process arguments in-place to avoid type mismatch issues + Object[] args = processArgsInPlace(joinPoint.getArgs(), async); // Extract queueable and resourceWeight parameters and validate boolean queueable = autoJobPostMapping.queueable(); @@ -229,79 +219,10 @@ public class AutoJobAspect { resourceWeight); } - /** - * Creates deep copies of arguments when needed to avoid mutating the original objects - * Particularly important for PDFFile objects that might be reused by Spring - * - * @param originalArgs The original arguments - * @param async Whether this is an async operation - * @return A new array with safely processed arguments - */ - private Object[] copyAndProcessArgs(Object[] originalArgs, boolean async) { - if (originalArgs == null || originalArgs.length == 0) { - return originalArgs; - } - - Object[] processedArgs = new Object[originalArgs.length]; - - // Copy all arguments - for (int i = 0; i < originalArgs.length; i++) { - Object arg = originalArgs[i]; - - if (arg instanceof PDFFile pdfFile) { - // Create a copy of PDFFile to avoid mutating the original - // Using direct property access instead of reflection for better performance - PDFFile pdfFileCopy = new PDFFile(); - pdfFileCopy.setFileId(pdfFile.getFileId()); - pdfFileCopy.setFileInput(pdfFile.getFileInput()); - - // Case 1: fileId is provided but no fileInput - if (pdfFileCopy.getFileInput() == null && pdfFileCopy.getFileId() != null) { - try { - log.debug("Using fileId {} to get file content", pdfFileCopy.getFileId()); - MultipartFile file = fileStorage.retrieveFile(pdfFileCopy.getFileId()); - pdfFileCopy.setFileInput(file); - } catch (Exception e) { - throw new RuntimeException( - "Failed to resolve file by ID: " + pdfFileCopy.getFileId(), e); - } - } - // Case 2: For async requests, we need to make a copy of the MultipartFile - else if (async && pdfFileCopy.getFileInput() != null) { - try { - log.debug("Making persistent copy of uploaded file for async processing"); - MultipartFile originalFile = pdfFileCopy.getFileInput(); - String fileId = fileStorage.storeFile(originalFile); - - // Store the fileId for later reference - pdfFileCopy.setFileId(fileId); - - // Replace the original MultipartFile with our persistent copy - MultipartFile persistentFile = fileStorage.retrieveFile(fileId); - pdfFileCopy.setFileInput(persistentFile); - - log.debug("Created persistent file copy with fileId: {}", fileId); - } catch (IOException e) { - throw new RuntimeException( - "Failed to create persistent copy of uploaded file", e); - } - } - - processedArgs[i] = pdfFileCopy; - } else { - // For non-PDFFile objects, just pass the original reference - // If other classes need copy-on-write, add them here - processedArgs[i] = arg; - } - } - - return processedArgs; - } /** - * Processes arguments in-place for testing purposes This is similar to our original - * implementation before introducing copy-on-write It's only used in test environments to - * maintain test compatibility + * Processes arguments in-place to handle file resolution and async file persistence. + * This approach avoids type mismatch issues by modifying the original objects directly. * * @param originalArgs The original arguments * @param async Whether this is an async operation diff --git a/common/src/main/java/stirling/software/common/model/job/JobResult.java b/common/src/main/java/stirling/software/common/model/job/JobResult.java index 1aa66d1a8..e4eb456fd 100644 --- a/common/src/main/java/stirling/software/common/model/job/JobResult.java +++ b/common/src/main/java/stirling/software/common/model/job/JobResult.java @@ -6,6 +6,8 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import com.fasterxml.jackson.annotation.JsonIgnore; + import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; @@ -28,6 +30,7 @@ public class JobResult { private String error; /** List of result files for jobs that produce files */ + @JsonIgnore private List resultFiles; /** Time when the job was created */ diff --git a/common/src/main/java/stirling/software/common/service/TaskManager.java b/common/src/main/java/stirling/software/common/service/TaskManager.java index 219ae4ac4..902b2bfd1 100644 --- a/common/src/main/java/stirling/software/common/service/TaskManager.java +++ b/common/src/main/java/stirling/software/common/service/TaskManager.java @@ -1,6 +1,5 @@ package stirling.software.common.service; -import io.github.pixee.security.ZipSecurity; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -21,6 +20,8 @@ import org.springframework.http.MediaType; import org.springframework.stereotype.Service; import org.springframework.web.multipart.MultipartFile; +import io.github.pixee.security.ZipSecurity; + import jakarta.annotation.PreDestroy; import lombok.extern.slf4j.Slf4j; @@ -361,7 +362,8 @@ public class TaskManager { MultipartFile zipFile = fileStorage.retrieveFile(zipFileId); try (ZipInputStream zipIn = - ZipSecurity.createHardenedInputStream(new ByteArrayInputStream(zipFile.getBytes()))) { + ZipSecurity.createHardenedInputStream( + new ByteArrayInputStream(zipFile.getBytes()))) { ZipEntry entry; while ((entry = zipIn.getNextEntry()) != null) { if (!entry.isDirectory()) { diff --git a/stirling-pdf/src/main/java/stirling/software/SPDF/config/CleanUrlInterceptor.java b/stirling-pdf/src/main/java/stirling/software/SPDF/config/CleanUrlInterceptor.java index eb9f2be33..088c0c0bf 100644 --- a/stirling-pdf/src/main/java/stirling/software/SPDF/config/CleanUrlInterceptor.java +++ b/stirling-pdf/src/main/java/stirling/software/SPDF/config/CleanUrlInterceptor.java @@ -29,7 +29,8 @@ public class CleanUrlInterceptor implements HandlerInterceptor { "type", "principal", "startDate", - "endDate"); + "endDate", + "async"); @Override public boolean preHandle(