refactor(core): centralize temp file handling in CompressController via TempFileManager (#4629)

# Description of Changes

## What was changed
- Introduced `TempFileManager` and injected it into `CompressController`
to centralize and control temporary file lifecycle.
- Replaced ad-hoc `Files.createTempFile(...)` usages with a new
`TempFile` abstraction:
- `compressImagesInPDF(...)` now returns a `TempFile` instead of a
`Path`.
- All intermediate artifacts (original/working/GS/QPDF outputs) are
created via `TempFile` and managed with try-with-resources where
applicable.
- Removed the mutable `List<Path> tempFiles` bookkeeping; cleanup is
handled by `TempFile.close()` and a single `finally` block that closes
all tracked `TempFile` instances.
- Updated save/copy calls to use `TempFile` accessors (`getPath()`,
`getAbsolutePath()`, `getFile()`).
- Hardened error handling:
- Ensured `TempFile` is closed on early exceptions (e.g., in
`compressImagesInPDF`).
- Ghostscript/QPDF helpers now encapsulate their output lifecycle and no
longer accept/require a temp file list.
- Minor Java refinements:
- Used pattern matching for `instanceof` (e.g., `if (ref instanceof
NestedImageReference nestedRef)`).
- Improved and wrapped long log messages for readability and
consistency.

## Why the change was made
- **Resource safety:** Prevent orphaned temp files and reduce
file-descriptor leaks under failure conditions or multi-step pipelines.
- **Consistency:** Establish a single, testable mechanism for temp file
creation, placement, and cleanup across compression flows.
- **Portability & stability:** Avoid Windows file-locking/delete-in-use
issues by using explicit close semantics and predictable lifetimes.
- **Maintainability:** Simplify control flow by removing ad-hoc temp
tracking and pushing lifecycle ownership into `TempFile`.

---

## Checklist

### General

- [x] I have read the [Contribution
Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md)
- [x] 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)
- [x] I have performed a self-review of my own code
- [x] 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.
This commit is contained in:
Ludy 2025-10-30 00:21:30 +01:00 committed by GitHub
parent e4cfb8befe
commit fdc8fab545
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 168 additions and 147 deletions

View File

@ -28,6 +28,17 @@ public class TempFileManager {
private final TempFileRegistry registry;
private final ApplicationProperties applicationProperties;
/**
* Create a managed temporary file that will be tracked by the TempFileManager.
*
* @param suffix The suffix for the temporary file
* @return The created temporary file wrapper
* @throws IOException If an I/O error occurs
*/
public TempFile createManagedTempFile(String suffix) throws IOException {
return new TempFile(this, suffix);
}
/**
* Create a temporary file with the Stirling-PDF prefix. The file is automatically registered
* with the registry.
@ -130,7 +141,6 @@ public class TempFileManager {
return deleted;
} catch (IOException e) {
log.warn("Failed to delete temp file: {}", path.toString(), e);
return false;
}
}
return false;

View File

@ -49,6 +49,8 @@ import stirling.software.common.util.ExceptionUtils;
import stirling.software.common.util.GeneralUtils;
import stirling.software.common.util.ProcessExecutor;
import stirling.software.common.util.ProcessExecutor.ProcessExecutorResult;
import stirling.software.common.util.TempFile;
import stirling.software.common.util.TempFileManager;
import stirling.software.common.util.WebResponseUtils;
@RestController
@ -60,6 +62,7 @@ public class CompressController {
private final CustomPDFDocumentFactory pdfDocumentFactory;
private final EndpointConfiguration endpointConfiguration;
private final TempFileManager tempFileManager;
private boolean isQpdfEnabled() {
return endpointConfiguration.isGroupEnabled("qpdf");
@ -97,13 +100,14 @@ public class CompressController {
long totalCompressedBytes = 0;
}
public Path compressImagesInPDF(
public TempFile compressImagesInPDF(
Path pdfFile, double scaleFactor, float jpegQuality, boolean convertToGrayscale)
throws Exception {
Path newCompressedPDF = Files.createTempFile("compressedPDF", ".pdf");
TempFile newCompressedPDF = tempFileManager.createManagedTempFile(".pdf");
long originalFileSize = Files.size(pdfFile);
log.info(
"Starting image compression with scale factor: {}, JPEG quality: {}, grayscale: {} on file size: {}",
"Starting image compression with scale factor: {}, JPEG quality: {}, grayscale: {}"
+ " on file size: {}",
scaleFactor,
jpegQuality,
convertToGrayscale,
@ -133,11 +137,11 @@ public class CompressController {
compressedVersions.clear();
uniqueImages.clear();
log.info("Saving compressed PDF to {}", newCompressedPDF.toString());
doc.save(newCompressedPDF.toString());
log.info("Saving compressed PDF to {}", newCompressedPDF.getPath());
doc.save(newCompressedPDF.getAbsolutePath());
// Log overall file size reduction
long compressedFileSize = Files.size(newCompressedPDF);
long compressedFileSize = Files.size(newCompressedPDF.getPath());
double overallReduction = 100.0 - ((compressedFileSize * 100.0) / originalFileSize);
log.info(
"Overall PDF compression: {} → {} (reduced by {}%)",
@ -145,6 +149,9 @@ public class CompressController {
GeneralUtils.formatBytes(compressedFileSize),
String.format("%.1f", overallReduction));
return newCompressedPDF;
} catch (Exception e) {
newCompressedPDF.close();
throw e;
}
}
@ -327,9 +334,8 @@ public class CompressController {
// Get original image from a reference
private PDImageXObject getOriginalImage(PDDocument doc, ImageReference ref) throws IOException {
if (ref instanceof NestedImageReference) {
if (ref instanceof NestedImageReference nestedRef) {
// Get the nested image from within a form XObject
NestedImageReference nestedRef = (NestedImageReference) ref;
PDPage page = doc.getPage(nestedRef.pageNum);
PDResources pageResources = page.getResources();
@ -405,9 +411,8 @@ public class CompressController {
// Replace a specific image reference with a compressed version
private void replaceImageReference(
PDDocument doc, ImageReference ref, PDImageXObject compressedImage) throws IOException {
if (ref instanceof NestedImageReference) {
if (ref instanceof NestedImageReference nestedRef) {
// Replace nested image within form XObject
NestedImageReference nestedRef = (NestedImageReference) ref;
PDPage page = doc.getPage(nestedRef.pageNum);
PDResources pageResources = page.getResources();
@ -444,7 +449,8 @@ public class CompressController {
int duplicatedImages = stats.totalImages - stats.uniqueImagesCount;
log.info(
"Image compression summary - Total unique: {}, Compressed: {}, Skipped: {}, Duplicates: {}, Nested: {}",
"Image compression summary - Total unique: {}, Compressed: {}, Skipped: {},"
+ " Duplicates: {}, Nested: {}",
stats.uniqueImagesCount,
stats.compressedImages,
stats.skippedImages,
@ -673,18 +679,20 @@ public class CompressController {
autoMode = true;
}
List<TempFile> tempFiles = new ArrayList<>();
// Create initial input file
Path originalFile = Files.createTempFile("original_", ".pdf");
inputFile.transferTo(originalFile.toFile());
TempFile originalTempFile = tempFileManager.createManagedTempFile(".pdf");
tempFiles.add(originalTempFile);
Path originalFile = originalTempFile.getPath();
inputFile.transferTo(originalTempFile.getFile());
long inputFileSize = Files.size(originalFile);
Path currentFile = Files.createTempFile("working_", ".pdf");
TempFile currentTempFile = tempFileManager.createManagedTempFile(".pdf");
tempFiles.add(currentTempFile);
Path currentFile = currentTempFile.getPath();
Files.copy(originalFile, currentFile, StandardCopyOption.REPLACE_EXISTING);
// Keep track of all temporary files for cleanup
List<Path> tempFiles = new ArrayList<>();
tempFiles.add(originalFile);
tempFiles.add(currentFile);
try {
if (autoMode) {
double sizeReductionRatio = expectedOutputSize / (double) inputFileSize;
@ -703,8 +711,7 @@ public class CompressController {
// Try Ghostscript first if available - for ANY compression level
if (isGhostscriptEnabled()) {
try {
applyGhostscriptCompression(
request, optimizeLevel, currentFile, tempFiles);
applyGhostscriptCompression(request, optimizeLevel, currentFile);
log.info("Ghostscript compression applied successfully");
ghostscriptSuccess = true;
} catch (IOException e) {
@ -715,7 +722,7 @@ public class CompressController {
// Fallback to QPDF if Ghostscript failed or not available (levels 1-3 only)
if (!ghostscriptSuccess && isQpdfEnabled() && optimizeLevel <= 3) {
try {
applyQpdfCompression(request, optimizeLevel, currentFile, tempFiles);
applyQpdfCompression(request, optimizeLevel, currentFile);
log.info("QPDF compression applied successfully");
} catch (IOException e) {
log.warn("QPDF compression also failed");
@ -724,7 +731,8 @@ public class CompressController {
if (!ghostscriptSuccess && !isQpdfEnabled()) {
log.info(
"No external compression tools available, using image compression only");
"No external compression tools available, using image compression"
+ " only");
}
externalCompressionApplied = true;
@ -751,7 +759,7 @@ public class CompressController {
};
log.info("Applying image compression with scale factor: {}", scaleFactor);
Path compressedImageFile =
TempFile compressedImageFile =
compressImagesInPDF(
currentFile,
scaleFactor,
@ -759,7 +767,7 @@ public class CompressController {
Boolean.TRUE.equals(convertToGrayscale));
tempFiles.add(compressedImageFile);
currentFile = compressedImageFile;
currentFile = compressedImageFile.getPath();
imageCompressionApplied = true;
}
@ -789,7 +797,8 @@ public class CompressController {
long finalFileSize = Files.size(currentFile);
if (finalFileSize >= inputFileSize) {
log.warn(
"Optimized file is larger than the original. Using the original file instead.");
"Optimized file is larger than the original. Using the original file"
+ " instead.");
currentFile = originalFile;
}
@ -802,10 +811,10 @@ public class CompressController {
} finally {
// Clean up all temporary files
for (Path tempFile : tempFiles) {
for (TempFile tempFile : tempFiles) {
try {
Files.deleteIfExists(tempFile);
} catch (IOException e) {
tempFile.close();
} catch (Exception e) {
log.warn("Failed to delete temporary file: {}", tempFile, e);
}
}
@ -814,15 +823,13 @@ public class CompressController {
// Run Ghostscript compression
private void applyGhostscriptCompression(
OptimizePdfRequest request, int optimizeLevel, Path currentFile, List<Path> tempFiles)
throws IOException {
OptimizePdfRequest request, int optimizeLevel, Path currentFile) throws IOException {
long preGsSize = Files.size(currentFile);
log.info("Pre-Ghostscript file size: {}", GeneralUtils.formatBytes(preGsSize));
// Create output file for Ghostscript
Path gsOutputFile = Files.createTempFile("gs_output_", ".pdf");
tempFiles.add(gsOutputFile);
try (TempFile gsOutputFile = tempFileManager.createManagedTempFile(".pdf")) {
Path gsOutputPath = gsOutputFile.getPath();
// Build Ghostscript command based on optimization level
List<String> command = new ArrayList<>();
@ -873,7 +880,7 @@ public class CompressController {
break;
}
command.add("-sOutputFile=" + gsOutputFile.toString());
command.add("-sOutputFile=" + gsOutputPath.toString());
command.add(currentFile.toString());
ProcessExecutorResult returnCode;
@ -884,15 +891,18 @@ public class CompressController {
if (returnCode.getRc() == 0) {
// Update current file to the Ghostscript output
Files.copy(gsOutputFile, currentFile, StandardCopyOption.REPLACE_EXISTING);
Files.copy(gsOutputPath, currentFile, StandardCopyOption.REPLACE_EXISTING);
long postGsSize = Files.size(currentFile);
double gsReduction = 100.0 - ((postGsSize * 100.0) / preGsSize);
log.info(
"Post-Ghostscript file size: {} (reduced by {}%)",
GeneralUtils.formatBytes(postGsSize), String.format("%.1f", gsReduction));
GeneralUtils.formatBytes(postGsSize),
String.format("%.1f", gsReduction));
} else {
log.warn("Ghostscript compression failed with return code: {}", returnCode.getRc());
log.warn(
"Ghostscript compression failed with return code: {}",
returnCode.getRc());
throw new IOException("Ghostscript compression failed");
}
@ -901,11 +911,11 @@ public class CompressController {
throw new IOException("Ghostscript compression failed", e);
}
}
}
// Run QPDF compression
private void applyQpdfCompression(
OptimizePdfRequest request, int optimizeLevel, Path currentFile, List<Path> tempFiles)
throws IOException {
OptimizePdfRequest request, int optimizeLevel, Path currentFile) throws IOException {
long preQpdfSize = Files.size(currentFile);
log.info("Pre-QPDF file size: {}", GeneralUtils.formatBytes(preQpdfSize));
@ -920,9 +930,8 @@ public class CompressController {
qpdfCompressionLevel = 9;
}
// Create output file for QPDF
Path qpdfOutputFile = Files.createTempFile("qpdf_output_", ".pdf");
tempFiles.add(qpdfOutputFile);
try (TempFile qpdfOutputFile = tempFileManager.createManagedTempFile(".pdf")) {
Path qpdfOutputPath = qpdfOutputFile.getPath();
// Build QPDF command
List<String> command = new ArrayList<>();
@ -938,7 +947,7 @@ public class CompressController {
command.add("--compress-streams=y");
command.add("--object-streams=generate");
command.add(currentFile.toString());
command.add(qpdfOutputFile.toString());
command.add(qpdfOutputPath.toString());
ProcessExecutorResult returnCode = null;
try {
@ -947,13 +956,14 @@ public class CompressController {
.runCommandWithOutputHandling(command);
// Update current file to the QPDF output
Files.copy(qpdfOutputFile, currentFile, StandardCopyOption.REPLACE_EXISTING);
Files.copy(qpdfOutputPath, currentFile, StandardCopyOption.REPLACE_EXISTING);
long postQpdfSize = Files.size(currentFile);
double qpdfReduction = 100.0 - ((postQpdfSize * 100.0) / preQpdfSize);
log.info(
"Post-QPDF file size: {} (reduced by {}%)",
GeneralUtils.formatBytes(postQpdfSize), String.format("%.1f", qpdfReduction));
GeneralUtils.formatBytes(postQpdfSize),
String.format("%.1f", qpdfReduction));
} catch (Exception e) {
if (returnCode != null && returnCode.getRc() != 3) {
@ -963,6 +973,7 @@ public class CompressController {
log.warn("QPDF compression failed, continuing with current file", e);
}
}
}
// Pick optimization level based on target size
private int determineOptimizeLevel(double sizeReductionRatio) {