diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 12a5da573..6e006423a 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -2,7 +2,10 @@ "permissions": { "allow": [ "Bash(chmod:*)", - "Bash(mkdir:*)" + "Bash(mkdir:*)", + "Bash(./gradlew:*)", + "Bash(grep:*)", + "Bash(cat:*)" ], "deny": [] } diff --git a/common/src/main/java/stirling/software/common/annotations/AutoJobPostMapping.java b/common/src/main/java/stirling/software/common/annotations/AutoJobPostMapping.java index 062f3e0a1..8fb729560 100644 --- a/common/src/main/java/stirling/software/common/annotations/AutoJobPostMapping.java +++ b/common/src/main/java/stirling/software/common/annotations/AutoJobPostMapping.java @@ -8,22 +8,22 @@ import org.springframework.web.bind.annotation.RequestMethod; /** * Shortcut for a POST endpoint that is executed through the Stirling "auto‑job" framework. - *

- * Behaviour notes: - *

- *

* - *

Unless stated otherwise an attribute only affects async execution.

+ *

Behaviour notes: + * + *

+ * + *

Unless stated otherwise an attribute only affects async execution. */ @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) @@ -31,42 +31,42 @@ import org.springframework.web.bind.annotation.RequestMethod; @RequestMapping(method = RequestMethod.POST) public @interface AutoJobPostMapping { - /** - * Alias for {@link RequestMapping#value} – the path mapping of the endpoint. - */ + /** Alias for {@link RequestMapping#value} – the path mapping of the endpoint. */ @AliasFor(annotation = RequestMapping.class, attribute = "value") String[] value() default {}; - /** - * MIME types this endpoint accepts. Defaults to {@code multipart/form-data}. - */ + /** MIME types this endpoint accepts. Defaults to {@code multipart/form-data}. */ @AliasFor(annotation = RequestMapping.class, attribute = "consumes") String[] consumes() default {"multipart/form-data"}; /** - * Maximum execution time in milliseconds before the job is aborted. - * A negative value means "use the application default". - *

Only honoured when {@code async=true}.

+ * Maximum execution time in milliseconds before the job is aborted. A negative value means "use + * the application default". + * + *

Only honoured when {@code async=true}. */ long timeout() default -1; /** - * Total number of attempts (initial + retries). Must be at least 1. - * Retries are executed with exponential back‑off. - *

Only honoured when {@code async=true}.

+ * Total number of attempts (initial + retries). Must be at least 1. Retries are executed + * with exponential back‑off. + * + *

Only honoured when {@code async=true}. */ int retryCount() default 1; /** * Record percentage / note updates so they can be retrieved via the REST status endpoint. - *

Only honoured when {@code async=true}.

+ * + *

Only honoured when {@code async=true}. */ boolean trackProgress() default true; /** - * If {@code true} the job may be placed in a queue instead of being rejected when resources - * are scarce. - *

Only honoured when {@code async=true}.

+ * If {@code true} the job may be placed in a queue instead of being rejected when resources are + * scarce. + * + *

Only honoured when {@code async=true}. */ boolean queueable() default false; diff --git a/common/src/main/java/stirling/software/common/config/TempFileConfiguration.java b/common/src/main/java/stirling/software/common/config/TempFileConfiguration.java index 4cb8ec189..8654959e2 100644 --- a/common/src/main/java/stirling/software/common/config/TempFileConfiguration.java +++ b/common/src/main/java/stirling/software/common/config/TempFileConfiguration.java @@ -1,6 +1,5 @@ package stirling.software.common.config; -import java.io.File; import java.nio.file.Files; import java.nio.file.Path; @@ -14,7 +13,6 @@ import jakarta.annotation.PostConstruct; import lombok.extern.slf4j.Slf4j; -import stirling.software.common.configuration.InstallationPathConfig; import stirling.software.common.util.TempFileRegistry; /** @@ -62,4 +60,4 @@ public class TempFileConfiguration { log.error("Failed to initialize temporary file configuration", e); } } -} \ No newline at end of file +} diff --git a/common/src/main/java/stirling/software/common/service/ResourceMonitor.java b/common/src/main/java/stirling/software/common/service/ResourceMonitor.java index 2791fff90..0e8073d8f 100644 --- a/common/src/main/java/stirling/software/common/service/ResourceMonitor.java +++ b/common/src/main/java/stirling/software/common/service/ResourceMonitor.java @@ -173,7 +173,9 @@ public class ResourceMonitor { log.info("System resource status changed from {} to {}", oldStatus, newStatus); log.info( "Current metrics - CPU: {}%, Memory: {}%, Free Memory: {} MB", - String.format("%.1f", cpuUsage * 100), String.format("%.1f", memoryUsage * 100), freeMemory / (1024 * 1024)); + String.format("%.1f", cpuUsage * 100), + String.format("%.1f", memoryUsage * 100), + freeMemory / (1024 * 1024)); } } catch (Exception e) { log.error("Error updating resource metrics: {}", e.getMessage(), e); diff --git a/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java b/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java index 78ef79c2a..ab786bf2b 100644 --- a/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java +++ b/common/src/main/java/stirling/software/common/service/TempFileCleanupService.java @@ -55,33 +55,36 @@ public class TempFileCleanupService { // Maximum recursion depth for directory traversal private static final int MAX_RECURSION_DEPTH = 5; - + // File patterns that identify our temp files - private static final Predicate IS_OUR_TEMP_FILE = fileName -> - fileName.startsWith("stirling-pdf-") || - fileName.startsWith("output_") || - fileName.startsWith("compressedPDF") || - fileName.startsWith("pdf-save-") || - fileName.startsWith("pdf-stream-") || - fileName.startsWith("PDFBox") || - fileName.startsWith("input_") || - fileName.startsWith("overlay-"); - + private static final Predicate IS_OUR_TEMP_FILE = + fileName -> + fileName.startsWith("stirling-pdf-") + || fileName.startsWith("output_") + || fileName.startsWith("compressedPDF") + || fileName.startsWith("pdf-save-") + || fileName.startsWith("pdf-stream-") + || fileName.startsWith("PDFBox") + || fileName.startsWith("input_") + || fileName.startsWith("overlay-"); + // File patterns that identify common system temp files - private static final Predicate IS_SYSTEM_TEMP_FILE = fileName -> - fileName.matches("lu\\d+[a-z0-9]*\\.tmp") || - fileName.matches("ocr_process\\d+") || - (fileName.startsWith("tmp") && !fileName.contains("jetty")) || - fileName.startsWith("OSL_PIPE_") || - (fileName.endsWith(".tmp") && !fileName.contains("jetty")); - + private static final Predicate IS_SYSTEM_TEMP_FILE = + fileName -> + fileName.matches("lu\\d+[a-z0-9]*\\.tmp") + || fileName.matches("ocr_process\\d+") + || (fileName.startsWith("tmp") && !fileName.contains("jetty")) + || fileName.startsWith("OSL_PIPE_") + || (fileName.endsWith(".tmp") && !fileName.contains("jetty")); + // File patterns that should be excluded from cleanup - private static final Predicate SHOULD_SKIP = fileName -> - fileName.contains("jetty") || - fileName.startsWith("jetty-") || - fileName.equals("proc") || - fileName.equals("sys") || - fileName.equals("dev"); + private static final Predicate SHOULD_SKIP = + fileName -> + fileName.contains("jetty") + || fileName.startsWith("jetty-") + || fileName.equals("proc") + || fileName.equals("sys") + || fileName.equals("dev"); @Autowired public TempFileCleanupService(TempFileRegistry registry, TempFileManager tempFileManager) { @@ -166,7 +169,7 @@ public class TempFileCleanupService { private void runStartupCleanup() { log.info("Running startup temporary file cleanup"); boolean containerMode = isContainerMode(); - + log.info( "Running in {} mode, using {} cleanup strategy", machineType, @@ -174,9 +177,9 @@ public class TempFileCleanupService { // For startup cleanup, we use a longer timeout for non-container environments long maxAgeMillis = containerMode ? 0 : 24 * 60 * 60 * 1000; // 0 or 24 hours - + int totalDeletedCount = cleanupUnregisteredFiles(containerMode, false, maxAgeMillis); - + log.info( "Startup cleanup complete. Deleted {} temporary files/directories", totalDeletedCount); @@ -184,66 +187,72 @@ public class TempFileCleanupService { /** * Clean up unregistered temporary files across all configured temp directories. - * + * * @param containerMode Whether we're in container mode (more aggressive cleanup) * @param isScheduled Whether this is a scheduled cleanup or startup cleanup * @param maxAgeMillis Maximum age of files to clean in milliseconds * @return Number of files deleted */ - private int cleanupUnregisteredFiles(boolean containerMode, boolean isScheduled, long maxAgeMillis) { + private int cleanupUnregisteredFiles( + boolean containerMode, boolean isScheduled, long maxAgeMillis) { AtomicInteger totalDeletedCount = new AtomicInteger(0); - + try { // Get all directories we need to clean Path systemTempPath = getSystemTempPath(); Path[] dirsToScan = { - systemTempPath, - Path.of(customTempDirectory), - Path.of(libreOfficeTempDir) + systemTempPath, Path.of(customTempDirectory), Path.of(libreOfficeTempDir) }; // Process each directory Arrays.stream(dirsToScan) - .filter(Files::exists) - .forEach(tempDir -> { - try { - String phase = isScheduled ? "scheduled" : "startup"; - log.info("Scanning directory for {} cleanup: {}", phase, tempDir); - - AtomicInteger dirDeletedCount = new AtomicInteger(0); - cleanupDirectoryStreaming( - tempDir, - containerMode, - 0, - maxAgeMillis, - isScheduled, - path -> { - dirDeletedCount.incrementAndGet(); - if (log.isDebugEnabled()) { - log.debug("Deleted temp file during {} cleanup: {}", phase, path); + .filter(Files::exists) + .forEach( + tempDir -> { + try { + String phase = isScheduled ? "scheduled" : "startup"; + log.info( + "Scanning directory for {} cleanup: {}", + phase, + tempDir); + + AtomicInteger dirDeletedCount = new AtomicInteger(0); + cleanupDirectoryStreaming( + tempDir, + containerMode, + 0, + maxAgeMillis, + isScheduled, + path -> { + dirDeletedCount.incrementAndGet(); + if (log.isDebugEnabled()) { + log.debug( + "Deleted temp file during {} cleanup: {}", + phase, + path); + } + }); + + int count = dirDeletedCount.get(); + totalDeletedCount.addAndGet(count); + if (count > 0) { + log.info( + "Cleaned up {} files/directories in {}", + count, + tempDir); + } + } catch (IOException e) { + log.error("Error during cleanup of directory: {}", tempDir, e); } - } - ); - - int count = dirDeletedCount.get(); - totalDeletedCount.addAndGet(count); - if (count > 0) { - log.info("Cleaned up {} files/directories in {}", count, tempDir); - } - } catch (IOException e) { - log.error("Error during cleanup of directory: {}", tempDir, e); - } - }); + }); } catch (Exception e) { log.error("Error during cleanup of unregistered files", e); } - + return totalDeletedCount.get(); } - /** - * Get the system temp directory path based on configuration or system property. - */ + /** Get the system temp directory path based on configuration or system property. */ private Path getSystemTempPath() { if (systemTempDir != null && !systemTempDir.isEmpty()) { return Path.of(systemTempDir); @@ -251,10 +260,8 @@ public class TempFileCleanupService { return Path.of(System.getProperty("java.io.tmpdir")); } } - - /** - * Determine if we're running in a container environment. - */ + + /** Determine if we're running in a container environment. */ private boolean isContainerMode() { return "Docker".equals(machineType) || "Kubernetes".equals(machineType); } @@ -271,13 +278,14 @@ public class TempFileCleanupService { * @throws IOException If an I/O error occurs */ private void cleanupDirectoryStreaming( - Path directory, - boolean containerMode, - int depth, + Path directory, + boolean containerMode, + int depth, long maxAgeMillis, boolean isScheduled, - Consumer onDeleteCallback) throws IOException { - + Consumer onDeleteCallback) + throws IOException { + // Check recursion depth limit if (depth > MAX_RECURSION_DEPTH) { log.warn("Maximum directory recursion depth reached for: {}", directory); @@ -287,66 +295,75 @@ public class TempFileCleanupService { // Use try-with-resources to ensure the stream is closed try (Stream pathStream = Files.list(directory)) { // Process files in a streaming fashion instead of materializing the whole list - pathStream.forEach(path -> { - try { - String fileName = path.getFileName().toString(); - - // Skip if file should be excluded - if (SHOULD_SKIP.test(fileName)) { - return; - } - - // Handle directories recursively - if (Files.isDirectory(path)) { + pathStream.forEach( + path -> { try { - cleanupDirectoryStreaming( - path, containerMode, depth + 1, maxAgeMillis, isScheduled, onDeleteCallback); - } catch (IOException e) { - log.warn("Error processing subdirectory: {}", path, e); - } - return; - } + String fileName = path.getFileName().toString(); - // Skip registered files - these are handled by TempFileManager - if (isScheduled && registry.contains(path.toFile())) { - return; - } - - // Check if this file should be deleted - if (shouldDeleteFile(path, fileName, containerMode, maxAgeMillis)) { - try { - Files.deleteIfExists(path); - onDeleteCallback.accept(path); - } catch (IOException e) { - // Handle locked files more gracefully - if (e.getMessage() != null && e.getMessage().contains("being used by another process")) { - log.debug("File locked, skipping delete: {}", path); - } else { - log.warn("Failed to delete temp file: {}", path, e); + // Skip if file should be excluded + if (SHOULD_SKIP.test(fileName)) { + return; } + + // Handle directories recursively + if (Files.isDirectory(path)) { + try { + cleanupDirectoryStreaming( + path, + containerMode, + depth + 1, + maxAgeMillis, + isScheduled, + onDeleteCallback); + } catch (IOException e) { + log.warn("Error processing subdirectory: {}", path, e); + } + return; + } + + // Skip registered files - these are handled by TempFileManager + if (registry.contains(path.toFile())) { + return; + } + + // Check if this file should be deleted + if (shouldDeleteFile(path, fileName, containerMode, maxAgeMillis)) { + try { + Files.deleteIfExists(path); + onDeleteCallback.accept(path); + } catch (IOException e) { + // Handle locked files more gracefully + if (e.getMessage() != null + && e.getMessage() + .contains("being used by another process")) { + log.debug("File locked, skipping delete: {}", path); + } else { + log.warn("Failed to delete temp file: {}", path, e); + } + } + } + } catch (Exception e) { + log.warn("Error processing path: {}", path, e); } - } - } catch (Exception e) { - log.warn("Error processing path: {}", path, e); - } - }); + }); } } - /** - * Determine if a file should be deleted based on its name, age, and other criteria. - */ - private boolean shouldDeleteFile(Path path, String fileName, boolean containerMode, long maxAgeMillis) { + /** Determine if a file should be deleted based on its name, age, and other criteria. */ + private boolean shouldDeleteFile( + Path path, String fileName, boolean containerMode, long maxAgeMillis) { // First check if it matches our known temp file patterns boolean isOurTempFile = IS_OUR_TEMP_FILE.test(fileName); boolean isSystemTempFile = IS_SYSTEM_TEMP_FILE.test(fileName); + + // Normal operation - check against temp file patterns boolean shouldDelete = isOurTempFile || (containerMode && isSystemTempFile); // Get file info for age checks long lastModified = 0; long currentTime = System.currentTimeMillis(); boolean isEmptyFile = false; - + try { lastModified = Files.getLastModifiedTime(path).toMillis(); // Special case for zero-byte files - these are often corrupted temp files @@ -362,8 +379,10 @@ public class TempFileCleanupService { log.debug("Could not check file info, skipping: {}", path); } - // Check file age against maxAgeMillis only if it's not an empty file that we've already decided to delete + // Check file age against maxAgeMillis only if it's not an empty file that we've already + // decided to delete if (!isEmptyFile && shouldDelete && maxAgeMillis > 0) { + // In normal mode, check age against maxAgeMillis shouldDelete = (currentTime - lastModified) > maxAgeMillis; } @@ -380,13 +399,12 @@ public class TempFileCleanupService { // For directories containing "libreoffice", delete all contents // but keep the directory itself for future use cleanupDirectoryStreaming( - dir, - isContainerMode(), - 0, - 0, // age doesn't matter for LibreOffice cleanup - false, - path -> log.debug("Cleaned up LibreOffice temp file: {}", path) - ); + dir, + isContainerMode(), + 0, + 0, // age doesn't matter for LibreOffice cleanup + false, + path -> log.debug("Cleaned up LibreOffice temp file: {}", path)); log.debug("Cleaned up LibreOffice temp directory contents: {}", dir); } } @@ -394,4 +412,4 @@ public class TempFileCleanupService { log.warn("Failed to clean up LibreOffice temp files", e); } } -} \ No newline at end of file +} diff --git a/common/src/main/java/stirling/software/common/util/TempFileManager.java b/common/src/main/java/stirling/software/common/util/TempFileManager.java index b427aee80..c20594864 100644 --- a/common/src/main/java/stirling/software/common/util/TempFileManager.java +++ b/common/src/main/java/stirling/software/common/util/TempFileManager.java @@ -37,7 +37,6 @@ public class TempFileManager { @Value("${stirling.tempfiles.max-age-hours:24}") private long maxAgeHours; - @Autowired public TempFileManager(TempFileRegistry registry) { this.registry = registry; @@ -227,15 +226,15 @@ public class TempFileManager { */ public Path registerLibreOfficeTempDir() throws IOException { Path loTempDir; - + // First check if explicitly configured if (libreOfficeTempDir != null && !libreOfficeTempDir.isEmpty()) { loTempDir = Path.of(libreOfficeTempDir); - } + } // Next check if we have a custom temp directory else if (customTempDirectory != null && !customTempDirectory.isEmpty()) { loTempDir = Path.of(customTempDirectory, "libreoffice"); - } + } // Fall back to system temp dir with our application prefix else { loTempDir = Path.of(System.getProperty("java.io.tmpdir"), "stirling-pdf-libreoffice"); diff --git a/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java b/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java index db0e8aaae..1e1633874 100644 --- a/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java +++ b/common/src/test/java/stirling/software/common/service/TempFileCleanupServiceTest.java @@ -10,19 +10,19 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; -import java.util.Arrays; import java.util.HashSet; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.MockitoAnnotations; import org.springframework.test.util.ReflectionTestUtils; @@ -67,7 +67,7 @@ public class TempFileCleanupServiceTest { ReflectionTestUtils.setField(cleanupService, "systemTempDir", systemTempDir.toString()); ReflectionTestUtils.setField(cleanupService, "customTempDirectory", customTempDir.toString()); ReflectionTestUtils.setField(cleanupService, "libreOfficeTempDir", libreOfficeTempDir.toString()); - ReflectionTestUtils.setField(cleanupService, "machineType", "Docker"); // Test in container mode + ReflectionTestUtils.setField(cleanupService, "machineType", "Standard"); // Regular mode ReflectionTestUtils.setField(cleanupService, "performStartupCleanup", false); // Disable auto-startup cleanup when(tempFileManager.getMaxAgeMillis()).thenReturn(3600000L); // 1 hour @@ -98,6 +98,9 @@ public class TempFileCleanupServiceTest { Path ourTempFile4 = Files.createFile(customTempDir.resolve("pdf-save-123-456.tmp")); Path ourTempFile5 = Files.createFile(libreOfficeTempDir.resolve("input_file.pdf")); + // Old temporary files + Path oldTempFile = Files.createFile(systemTempDir.resolve("output_old.pdf")); + // System temp files that should be cleaned in container mode Path sysTempFile1 = Files.createFile(systemTempDir.resolve("lu123abc.tmp")); Path sysTempFile2 = Files.createFile(customTempDir.resolve("ocr_process123")); @@ -118,45 +121,217 @@ public class TempFileCleanupServiceTest { // Configure mock registry to say these files aren't registered when(registry.contains(any(File.class))).thenReturn(false); - // Create a file older than threshold - Path oldFile = Files.createFile(systemTempDir.resolve("output_old.pdf")); - Files.setLastModifiedTime(oldFile, FileTime.from( - Files.getLastModifiedTime(oldFile).toMillis() - 5000000, - TimeUnit.MILLISECONDS)); + // The set of files that will be deleted in our test + Set deletedFiles = new HashSet<>(); - // Act - invokeCleanupDirectoryStreaming(systemTempDir, true, 0, 3600000); - invokeCleanupDirectoryStreaming(customTempDir, true, 0, 3600000); - invokeCleanupDirectoryStreaming(libreOfficeTempDir, true, 0, 3600000); + // Use MockedStatic to mock Files operations + try (MockedStatic mockedFiles = mockStatic(Files.class)) { + // Mock Files.list for each directory we'll process + mockedFiles.when(() -> Files.list(eq(systemTempDir))) + .thenReturn(Stream.of( + ourTempFile1, ourTempFile2, oldTempFile, sysTempFile1, + jettyFile1, jettyFile2, regularFile, emptyFile, nestedDir)); + + mockedFiles.when(() -> Files.list(eq(customTempDir))) + .thenReturn(Stream.of(ourTempFile3, ourTempFile4, sysTempFile2, sysTempFile3)); + + mockedFiles.when(() -> Files.list(eq(libreOfficeTempDir))) + .thenReturn(Stream.of(ourTempFile5)); + + mockedFiles.when(() -> Files.list(eq(nestedDir))) + .thenReturn(Stream.of(nestedTempFile)); + + // Configure Files.isDirectory for each path + mockedFiles.when(() -> Files.isDirectory(eq(nestedDir))).thenReturn(true); + mockedFiles.when(() -> Files.isDirectory(any(Path.class))).thenReturn(false); + + // Configure Files.exists to return true for all paths + mockedFiles.when(() -> Files.exists(any(Path.class))).thenReturn(true); + + // Configure Files.getLastModifiedTime to return different times based on file names + mockedFiles.when(() -> Files.getLastModifiedTime(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + String fileName = path.getFileName().toString(); + + // For files with "old" in the name, return a timestamp older than maxAgeMillis + if (fileName.contains("old")) { + return FileTime.fromMillis(System.currentTimeMillis() - 5000000); + } + // For empty.tmp file, return a timestamp older than 5 minutes (for empty file test) + else if (fileName.equals("empty.tmp")) { + return FileTime.fromMillis(System.currentTimeMillis() - 6 * 60 * 1000); + } + // For all other files, return a recent timestamp + else { + return FileTime.fromMillis(System.currentTimeMillis() - 60000); // 1 minute ago + } + }); + + // Configure Files.size to return different sizes based on file names + mockedFiles.when(() -> Files.size(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + String fileName = path.getFileName().toString(); + + // Return 0 bytes for the empty file + if (fileName.equals("empty.tmp")) { + return 0L; + } + // Return normal size for all other files + else { + return 1024L; // 1 KB + } + }); + + // For deleteIfExists, track which files would be deleted + mockedFiles.when(() -> Files.deleteIfExists(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + deletedFiles.add(path); + return true; + }); + + // Act - set containerMode to false for this test + invokeCleanupDirectoryStreaming(systemTempDir, false, 0, 3600000); + invokeCleanupDirectoryStreaming(customTempDir, false, 0, 3600000); + invokeCleanupDirectoryStreaming(libreOfficeTempDir, false, 0, 3600000); + + // Assert - Only old temp files and empty files should be deleted + assertTrue(deletedFiles.contains(oldTempFile), "Old temp file should be deleted"); + assertTrue(deletedFiles.contains(emptyFile), "Empty file should be deleted"); + + // Regular temp files should not be deleted because they're too new + assertFalse(deletedFiles.contains(ourTempFile1), "Recent temp file should be preserved"); + assertFalse(deletedFiles.contains(ourTempFile2), "Recent temp file should be preserved"); + assertFalse(deletedFiles.contains(ourTempFile3), "Recent temp file should be preserved"); + assertFalse(deletedFiles.contains(ourTempFile4), "Recent temp file should be preserved"); + assertFalse(deletedFiles.contains(ourTempFile5), "Recent temp file should be preserved"); + + // System temp files should not be deleted in non-container mode + assertFalse(deletedFiles.contains(sysTempFile1), "System temp file should be preserved in non-container mode"); + assertFalse(deletedFiles.contains(sysTempFile2), "System temp file should be preserved in non-container mode"); + assertFalse(deletedFiles.contains(sysTempFile3), "System temp file should be preserved in non-container mode"); + + // Jetty files and regular files should never be deleted + assertFalse(deletedFiles.contains(jettyFile1), "Jetty file should be preserved"); + assertFalse(deletedFiles.contains(jettyFile2), "File with jetty in name should be preserved"); + assertFalse(deletedFiles.contains(regularFile), "Regular file should be preserved"); + } + } - // Assert - Our temp files and system temp files should be deleted (if old enough) - assertFalse(Files.exists(oldFile), "Old temp file should be deleted"); - assertTrue(Files.exists(ourTempFile1), "Recent temp file should be preserved"); - assertTrue(Files.exists(sysTempFile1), "Recent system temp file should be preserved"); + @Test + public void testContainerModeCleanup() throws IOException { + // Arrange - Create various temp files + Path ourTempFile = Files.createFile(systemTempDir.resolve("output_123.pdf")); + Path sysTempFile = Files.createFile(systemTempDir.resolve("lu123abc.tmp")); + Path regularFile = Files.createFile(systemTempDir.resolve("important.txt")); - // Jetty files and regular files should never be deleted - assertTrue(Files.exists(jettyFile1), "Jetty file should be preserved"); - assertTrue(Files.exists(jettyFile2), "File with jetty in name should be preserved"); - assertTrue(Files.exists(regularFile), "Regular file should be preserved"); + // Configure mock registry to say these files aren't registered + when(registry.contains(any(File.class))).thenReturn(false); + + // The set of files that will be deleted in our test + Set deletedFiles = new HashSet<>(); + + // Use MockedStatic to mock Files operations + try (MockedStatic mockedFiles = mockStatic(Files.class)) { + // Mock Files.list for systemTempDir + mockedFiles.when(() -> Files.list(eq(systemTempDir))) + .thenReturn(Stream.of(ourTempFile, sysTempFile, regularFile)); + + // Configure Files.isDirectory + mockedFiles.when(() -> Files.isDirectory(any(Path.class))).thenReturn(false); + + // Configure Files.exists + mockedFiles.when(() -> Files.exists(any(Path.class))).thenReturn(true); + + // Configure Files.getLastModifiedTime to return recent timestamps + mockedFiles.when(() -> Files.getLastModifiedTime(any(Path.class))) + .thenReturn(FileTime.fromMillis(System.currentTimeMillis() - 60000)); // 1 minute ago + + // Configure Files.size to return normal size + mockedFiles.when(() -> Files.size(any(Path.class))) + .thenReturn(1024L); // 1 KB + + // For deleteIfExists, track which files would be deleted + mockedFiles.when(() -> Files.deleteIfExists(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + deletedFiles.add(path); + return true; + }); + + // Act - set containerMode to true and maxAgeMillis to 0 for container startup cleanup + invokeCleanupDirectoryStreaming(systemTempDir, true, 0, 0); + + // Assert - In container mode, both our temp files and system temp files should be deleted + // regardless of age (when maxAgeMillis is 0) + assertTrue(deletedFiles.contains(ourTempFile), "Our temp file should be deleted in container mode"); + assertTrue(deletedFiles.contains(sysTempFile), "System temp file should be deleted in container mode"); + assertFalse(deletedFiles.contains(regularFile), "Regular file should be preserved"); + } } @Test public void testEmptyFileHandling() throws IOException { // Arrange - Create an empty file Path emptyFile = Files.createFile(systemTempDir.resolve("empty.tmp")); - // Make it "old enough" to be deleted (>5 minutes) - Files.setLastModifiedTime(emptyFile, FileTime.from( - Files.getLastModifiedTime(emptyFile).toMillis() - 6 * 60 * 1000, - TimeUnit.MILLISECONDS)); - - // Configure mock registry to say this file isn't registered + Path recentEmptyFile = Files.createFile(systemTempDir.resolve("recent_empty.tmp")); + + // Configure mock registry to say these files aren't registered when(registry.contains(any(File.class))).thenReturn(false); + + // The set of files that will be deleted in our test + Set deletedFiles = new HashSet<>(); - // Act - invokeCleanupDirectoryStreaming(systemTempDir, true, 0, 3600000); - - // Assert - assertFalse(Files.exists(emptyFile), "Empty file older than 5 minutes should be deleted"); + // Use MockedStatic to mock Files operations + try (MockedStatic mockedFiles = mockStatic(Files.class)) { + // Mock Files.list for systemTempDir + mockedFiles.when(() -> Files.list(eq(systemTempDir))) + .thenReturn(Stream.of(emptyFile, recentEmptyFile)); + + // Configure Files.isDirectory + mockedFiles.when(() -> Files.isDirectory(any(Path.class))).thenReturn(false); + + // Configure Files.exists + mockedFiles.when(() -> Files.exists(any(Path.class))).thenReturn(true); + + // Configure Files.getLastModifiedTime to return different times based on file names + mockedFiles.when(() -> Files.getLastModifiedTime(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + String fileName = path.getFileName().toString(); + + if (fileName.equals("empty.tmp")) { + // More than 5 minutes old + return FileTime.fromMillis(System.currentTimeMillis() - 6 * 60 * 1000); + } else { + // Less than 5 minutes old + return FileTime.fromMillis(System.currentTimeMillis() - 2 * 60 * 1000); + } + }); + + // Configure Files.size to return 0 for empty files + mockedFiles.when(() -> Files.size(any(Path.class))) + .thenReturn(0L); + + // For deleteIfExists, track which files would be deleted + mockedFiles.when(() -> Files.deleteIfExists(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + deletedFiles.add(path); + return true; + }); + + // Act + invokeCleanupDirectoryStreaming(systemTempDir, false, 0, 3600000); + + // Assert + assertTrue(deletedFiles.contains(emptyFile), + "Empty file older than 5 minutes should be deleted"); + assertFalse(deletedFiles.contains(recentEmptyFile), + "Empty file newer than 5 minutes should not be deleted"); + } } @Test @@ -168,23 +343,79 @@ public class TempFileCleanupServiceTest { Path tempFile1 = Files.createFile(dir1.resolve("output_1.pdf")); Path tempFile2 = Files.createFile(dir2.resolve("output_2.pdf")); - Path tempFile3 = Files.createFile(dir3.resolve("output_3.pdf")); - - // Make the deepest file old enough to be deleted - Files.setLastModifiedTime(tempFile3, FileTime.from( - Files.getLastModifiedTime(tempFile3).toMillis() - 5000000, - TimeUnit.MILLISECONDS)); + Path tempFile3 = Files.createFile(dir3.resolve("output_old_3.pdf")); // Configure mock registry to say these files aren't registered when(registry.contains(any(File.class))).thenReturn(false); + + // The set of files that will be deleted in our test + Set deletedFiles = new HashSet<>(); - // Act - invokeCleanupDirectoryStreaming(systemTempDir, true, 0, 3600000); - - // Assert - assertTrue(Files.exists(tempFile1), "Recent temp file should be preserved"); - assertTrue(Files.exists(tempFile2), "Recent temp file should be preserved"); - assertFalse(Files.exists(tempFile3), "Old temp file in nested directory should be deleted"); + // Use MockedStatic to mock Files operations + try (MockedStatic mockedFiles = mockStatic(Files.class)) { + // Mock Files.list for each directory + mockedFiles.when(() -> Files.list(eq(systemTempDir))) + .thenReturn(Stream.of(dir1)); + + mockedFiles.when(() -> Files.list(eq(dir1))) + .thenReturn(Stream.of(tempFile1, dir2)); + + mockedFiles.when(() -> Files.list(eq(dir2))) + .thenReturn(Stream.of(tempFile2, dir3)); + + mockedFiles.when(() -> Files.list(eq(dir3))) + .thenReturn(Stream.of(tempFile3)); + + // Configure Files.isDirectory for each path + mockedFiles.when(() -> Files.isDirectory(eq(dir1))).thenReturn(true); + mockedFiles.when(() -> Files.isDirectory(eq(dir2))).thenReturn(true); + mockedFiles.when(() -> Files.isDirectory(eq(dir3))).thenReturn(true); + mockedFiles.when(() -> Files.isDirectory(eq(tempFile1))).thenReturn(false); + mockedFiles.when(() -> Files.isDirectory(eq(tempFile2))).thenReturn(false); + mockedFiles.when(() -> Files.isDirectory(eq(tempFile3))).thenReturn(false); + + // Configure Files.exists to return true for all paths + mockedFiles.when(() -> Files.exists(any(Path.class))).thenReturn(true); + + // Configure Files.getLastModifiedTime to return different times based on file names + mockedFiles.when(() -> Files.getLastModifiedTime(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + String fileName = path.getFileName().toString(); + + if (fileName.contains("old")) { + // Old file + return FileTime.fromMillis(System.currentTimeMillis() - 5000000); + } else { + // Recent file + return FileTime.fromMillis(System.currentTimeMillis() - 60000); + } + }); + + // Configure Files.size to return normal size + mockedFiles.when(() -> Files.size(any(Path.class))) + .thenReturn(1024L); + + // For deleteIfExists, track which files would be deleted + mockedFiles.when(() -> Files.deleteIfExists(any(Path.class))) + .thenAnswer(invocation -> { + Path path = invocation.getArgument(0); + deletedFiles.add(path); + return true; + }); + + // Act + invokeCleanupDirectoryStreaming(systemTempDir, false, 0, 3600000); + + // Debug - print what was deleted + System.out.println("Deleted files: " + deletedFiles); + System.out.println("Looking for: " + tempFile3); + + // Assert + assertFalse(deletedFiles.contains(tempFile1), "Recent temp file should be preserved"); + assertFalse(deletedFiles.contains(tempFile2), "Recent temp file should be preserved"); + assertTrue(deletedFiles.contains(tempFile3), "Old temp file in nested directory should be deleted"); + } } /** @@ -197,7 +428,7 @@ public class TempFileCleanupServiceTest { AtomicInteger deleteCount = new AtomicInteger(0); Consumer deleteCallback = path -> deleteCount.incrementAndGet(); - // Get the new method with updated signature + // Get the method with updated signature var method = TempFileCleanupService.class.getDeclaredMethod( "cleanupDirectoryStreaming", Path.class, boolean.class, int.class, long.class, boolean.class, Consumer.class); @@ -209,4 +440,9 @@ public class TempFileCleanupServiceTest { throw new RuntimeException("Error invoking cleanupDirectoryStreaming", e); } } + + // Matcher for exact path equality + private static Path eq(Path path) { + return argThat(arg -> arg != null && arg.equals(path)); + } } \ No newline at end of file diff --git a/proprietary/src/main/java/stirling/software/proprietary/config/HttpRequestAuditPublisher.java b/proprietary/src/main/java/stirling/software/proprietary/config/HttpRequestAuditPublisher.java index e69de29bb..8b1378917 100644 --- a/proprietary/src/main/java/stirling/software/proprietary/config/HttpRequestAuditPublisher.java +++ b/proprietary/src/main/java/stirling/software/proprietary/config/HttpRequestAuditPublisher.java @@ -0,0 +1 @@ + diff --git a/stirling-pdf/src/main/java/stirling/software/SPDF/UnoconvServer.java b/stirling-pdf/src/main/java/stirling/software/SPDF/UnoconvServer.java index a010720ed..37d27530a 100644 --- a/stirling-pdf/src/main/java/stirling/software/SPDF/UnoconvServer.java +++ b/stirling-pdf/src/main/java/stirling/software/SPDF/UnoconvServer.java @@ -11,7 +11,9 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; import io.github.pixee.security.SystemCommand; + import lombok.extern.slf4j.Slf4j; + import stirling.software.common.service.TempFileCleanupService; import stirling.software.common.util.ApplicationContextProvider; import stirling.software.common.util.TempFileManager; @@ -28,7 +30,7 @@ public class UnoconvServer { private long lastActivityTime; private Process process; private Path tempDir; - + private final TempFileManager tempFileManager; private final TempFileCleanupService cleanupService; @@ -43,7 +45,7 @@ public class UnoconvServer { // If INSTANCE is not set through Spring, try to get it from the ApplicationContext if (INSTANCE == null) { INSTANCE = ApplicationContextProvider.getBean(UnoconvServer.class); - + if (INSTANCE == null) { log.warn("Creating UnoconvServer without Spring context"); INSTANCE = new UnoconvServer(null, null); @@ -75,14 +77,14 @@ public class UnoconvServer { tempDir = tempFileManager.registerLibreOfficeTempDir(); log.info("Created unoconv temp directory: {}", tempDir); } - + String command; if (tempDir != null) { command = "unoconv-server --user-profile " + tempDir.toString(); } else { command = "unoconv-server"; } - + // Start the server process process = SystemCommand.runCommand(Runtime.getRuntime(), command); lastActivityTime = System.currentTimeMillis(); @@ -95,7 +97,7 @@ public class UnoconvServer { long idleTime = System.currentTimeMillis() - lastActivityTime; if (idleTime >= ACTIVITY_TIMEOUT) { process.destroy(); - + if (cleanupService != null) { cleanupService.cleanupLibreOfficeTempFiles(); } @@ -137,16 +139,14 @@ public class UnoconvServer { if (process != null && process.isAlive()) { process.destroy(); } - + if (cleanupService != null) { cleanupService.cleanupLibreOfficeTempFiles(); } } - - /** - * Notify that unoconv is being used, to reset the inactivity timer. - */ + + /** Notify that unoconv is being used, to reset the inactivity timer. */ public void notifyActivity() { lastActivityTime = System.currentTimeMillis(); } -} \ No newline at end of file +} diff --git a/stirling-pdf/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java b/stirling-pdf/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java index 2e7a197de..f4b27f33d 100644 --- a/stirling-pdf/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java +++ b/stirling-pdf/src/main/java/stirling/software/SPDF/config/EndpointConfiguration.java @@ -10,6 +10,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; import lombok.extern.slf4j.Slf4j; + import stirling.software.common.model.ApplicationProperties; @Service diff --git a/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java b/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java index 00b08d2bb..93061b570 100644 --- a/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java +++ b/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/OCRController.java @@ -35,8 +35,6 @@ import stirling.software.common.service.CustomPDFDocumentFactory; import stirling.software.common.util.ProcessExecutor; import stirling.software.common.util.ProcessExecutor.ProcessExecutorResult; import stirling.software.common.util.TempFileManager; -import stirling.software.common.util.TempFileUtil; -import stirling.software.common.util.TempFileUtil.TempFile; @RestController @RequestMapping("/api/v1/misc") @@ -80,52 +78,55 @@ public class OCRController { // Create a temp directory using TempFileManager directly Path tempDirPath = tempFileManager.createTempDirectory(); File tempDir = tempDirPath.toFile(); - + try { File tempInputFile = new File(tempDir, "input.pdf"); File tempOutputDir = new File(tempDir, "output"); File tempImagesDir = new File(tempDir, "images"); File finalOutputFile = new File(tempDir, "final_output.pdf"); - + // Create directories tempOutputDir.mkdirs(); tempImagesDir.mkdirs(); - + // Save input file inputFile.transferTo(tempInputFile); - + PDFMergerUtility merger = new PDFMergerUtility(); merger.setDestinationFileName(finalOutputFile.toString()); - + try (PDDocument document = pdfDocumentFactory.load(tempInputFile)) { PDFRenderer pdfRenderer = new PDFRenderer(document); int pageCount = document.getNumberOfPages(); - + for (int pageNum = 0; pageNum < pageCount; pageNum++) { PDPage page = document.getPage(pageNum); boolean hasText = false; - + // Check for existing text try (PDDocument tempDoc = new PDDocument()) { tempDoc.addPage(page); PDFTextStripper stripper = new PDFTextStripper(); hasText = !stripper.getText(tempDoc).trim().isEmpty(); } - - boolean shouldOcr = switch (ocrType) { - case "skip-text" -> !hasText; - case "force-ocr" -> true; - default -> true; - }; - - File pageOutputPath = new File(tempOutputDir, String.format("page_%d.pdf", pageNum)); - + + boolean shouldOcr = + switch (ocrType) { + case "skip-text" -> !hasText; + case "force-ocr" -> true; + default -> true; + }; + + File pageOutputPath = + new File(tempOutputDir, String.format("page_%d.pdf", pageNum)); + if (shouldOcr) { // Convert page to image BufferedImage image = pdfRenderer.renderImageWithDPI(pageNum, 300); - File imagePath = new File(tempImagesDir, String.format("page_%d.png", pageNum)); + File imagePath = + new File(tempImagesDir, String.format("page_%d.png", pageNum)); ImageIO.write(image, "png", imagePath); - + // Build OCR command List command = new ArrayList<>(); command.add("tesseract"); @@ -137,19 +138,25 @@ public class OCRController { command.add(String.join("+", languages)); // Always output PDF command.add("pdf"); - + // Use ProcessExecutor to run tesseract command try { - ProcessExecutorResult result = ProcessExecutor.getInstance(ProcessExecutor.Processes.TESSERACT) - .runCommandWithOutputHandling(command); - - log.debug("Tesseract OCR completed for page {} with exit code {}", - pageNum, result.getRc()); - + ProcessExecutorResult result = + ProcessExecutor.getInstance(ProcessExecutor.Processes.TESSERACT) + .runCommandWithOutputHandling(command); + + log.debug( + "Tesseract OCR completed for page {} with exit code {}", + pageNum, + result.getRc()); + // Add OCR'd PDF to merger merger.addSource(pageOutputPath); } catch (IOException | InterruptedException e) { - log.error("Error processing page {} with tesseract: {}", pageNum, e.getMessage()); + log.error( + "Error processing page {} with tesseract: {}", + pageNum, + e.getMessage()); // If OCR fails, fall back to the original page try (PDDocument pageDoc = new PDDocument()) { pageDoc.addPage(page); @@ -167,17 +174,17 @@ public class OCRController { } } } - + // Merge all pages into final PDF merger.mergeDocuments(null); - + // Read the final PDF file byte[] pdfContent = java.nio.file.Files.readAllBytes(finalOutputFile.toPath()); String outputFilename = Filenames.toSimpleFileName(inputFile.getOriginalFilename()) .replaceFirst("[.][^.]+$", "") + "_OCR.pdf"; - + return ResponseEntity.ok() .header( "Content-Disposition", @@ -189,7 +196,7 @@ public class OCRController { tempFileManager.deleteTempDirectory(tempDirPath); } } - + private void addFileToZip(File file, String filename, ZipOutputStream zipOut) throws IOException { if (!file.exists()) { @@ -207,4 +214,4 @@ public class OCRController { zipOut.closeEntry(); } } -} \ No newline at end of file +} diff --git a/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/RepairController.java b/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/RepairController.java index 64398c422..6847f2222 100644 --- a/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/RepairController.java +++ b/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/RepairController.java @@ -1,8 +1,6 @@ package stirling.software.SPDF.controller.api.misc; import java.io.IOException; -import stirling.software.common.util.TempFileManager; -import stirling.software.common.util.TempFileUtil; import java.util.ArrayList; import java.util.List; @@ -23,6 +21,8 @@ import stirling.software.common.model.api.PDFFile; import stirling.software.common.service.CustomPDFDocumentFactory; import stirling.software.common.util.ProcessExecutor; import stirling.software.common.util.ProcessExecutor.ProcessExecutorResult; +import stirling.software.common.util.TempFileManager; +import stirling.software.common.util.TempFileUtil; import stirling.software.common.util.WebResponseUtils; @RestController @@ -44,7 +44,7 @@ public class RepairController { public ResponseEntity repairPdf(@ModelAttribute PDFFile file) throws IOException, InterruptedException { MultipartFile inputFile = file.getFileInput(); - + // Use TempFileUtil.TempFile with try-with-resources for automatic cleanup try (TempFileUtil.TempFile tempFile = new TempFileUtil.TempFile(tempFileManager, ".pdf")) { // Save the uploaded file to the temporary location diff --git a/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java b/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java index a2713e06a..963bd0214 100644 --- a/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java +++ b/stirling-pdf/src/main/java/stirling/software/SPDF/controller/api/misc/StampController.java @@ -6,8 +6,6 @@ import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import stirling.software.common.util.TempFileManager; -import stirling.software.common.util.TempFileUtil; import java.util.List; import javax.imageio.ImageIO; @@ -41,6 +39,8 @@ import lombok.RequiredArgsConstructor; import stirling.software.SPDF.model.api.misc.AddStampRequest; import stirling.software.common.service.CustomPDFDocumentFactory; +import stirling.software.common.util.TempFileManager; +import stirling.software.common.util.TempFileUtil; import stirling.software.common.util.WebResponseUtils; @RestController @@ -51,8 +51,6 @@ public class StampController { private final CustomPDFDocumentFactory pdfDocumentFactory; private final TempFileManager tempFileManager; - - @PostMapping(consumes = "multipart/form-data", value = "/add-stamp") @Operation( @@ -192,9 +190,10 @@ public class StampController { if (!"".equals(resourceDir)) { ClassPathResource classPathResource = new ClassPathResource(resourceDir); String fileExtension = resourceDir.substring(resourceDir.lastIndexOf(".")); - + // Use TempFileUtil.TempFile with try-with-resources for automatic cleanup - try (TempFileUtil.TempFile tempFileWrapper = new TempFileUtil.TempFile(tempFileManager, fileExtension)) { + try (TempFileUtil.TempFile tempFileWrapper = + new TempFileUtil.TempFile(tempFileManager, fileExtension)) { File tempFile = tempFileWrapper.getFile(); try (InputStream is = classPathResource.getInputStream(); FileOutputStream os = new FileOutputStream(tempFile)) {