mirror of
https://github.com/Frooodle/Stirling-PDF.git
synced 2026-02-17 13:52:14 +01:00
refactor(resource): improve resource management and exception safety across controllers and utilities (#5350)
TLDR: - Use try-with-resources for InputStreams, PDDocument, TempFile, and ByteArrayOutputStream to ensure proper cleanup - Refactor PDF manipulation methods to handle exceptions and resource closure more robustly - Simplify session sorting logic in SessionPersistentRegistry - Add missing resource cleanup in MergeController and SplitPdfBySectionsController - Update PrintFileController to close streams and documents safely - Add unit test for SplitPdfBySizeController # Description of Changes This pull request introduces several improvements to resource management and error handling throughout the codebase, especially for temporary files and PDF document objects. The changes help prevent resource leaks by ensuring files and documents are properly closed or deleted in the event of errors or after use. Notable updates include the use of try-with-resources, custom AutoCloseable wrappers, and enhanced error handling logic. **Resource Management and Error Handling Improvements** * Added try-catch blocks to delete temporary files if an exception occurs during file conversion in `GeneralUtils.java`, ensuring no orphaned temp files are left behind * Introduced the `TempFile` AutoCloseable helper class in `InvertFullColorStrategy.java`, and refactored the PDF processing logic to use try-with-resources for both temporary files and `PDDocument` objects, ensuring proper cleanup * Improved error handling in `MergeController.java` by ensuring that partially created merged documents are closed if an error occurs during the merge process * Enhanced the splitting logic in `SplitPdfBySectionsController.java` to close any partially created `PDDocument` objects if an exception is thrown, preventing resource leaks **Refactoring for Safer Document Handling** * Refactored `handleSplitBySize` in `SplitPdfBySizeController.java` to use a custom `DocHolder` AutoCloseable wrapper for managing the lifecycle of `PDDocument` objects, and updated all related logic to use this wrapper, improving code safety and clarity * Updated `handleSplitByPageCount` in `SplitPdfBySizeController.java` to ensure `PDDocument` objects are set to null after being saved and closed, reducing the risk of operating on closed resources <!-- Please provide a summary of the changes, including: - What was changed - Why the change was made - Any challenges encountered Closes #(issue_number) --> --- ## 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) ### Translations (if applicable) - [ ] I ran [`scripts/counter_translation.py`](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/docs/counter_translation.md) ### 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. --------- Signed-off-by: Balázs Szücs <bszucs1209@gmail.com>
This commit is contained in:
@@ -91,6 +91,14 @@ public class GeneralUtils {
|
||||
while ((bytesRead = inputStream.read(buffer)) != -1) {
|
||||
outputStream.write(buffer, 0, bytesRead);
|
||||
}
|
||||
} catch (IOException e) {
|
||||
if (tempFile.exists()) {
|
||||
try {
|
||||
Files.delete(tempFile.toPath());
|
||||
} catch (IOException ignored) {
|
||||
}
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
return tempFile;
|
||||
}
|
||||
@@ -499,6 +507,12 @@ public class GeneralUtils {
|
||||
while ((bytesRead = in.read(buffer)) != -1) {
|
||||
out.write(buffer, 0, bytesRead);
|
||||
}
|
||||
} catch (IOException e) {
|
||||
try {
|
||||
Files.deleteIfExists(tempFile);
|
||||
} catch (IOException ignored) {
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
return tempFile.toFile();
|
||||
}
|
||||
|
||||
@@ -63,15 +63,16 @@ public class ImageProcessingUtils {
|
||||
} else {
|
||||
int width = image.getWidth();
|
||||
int height = image.getHeight();
|
||||
int[] pixels = new int[width * height];
|
||||
|
||||
image.getRGB(0, 0, width, height, pixels, 0, width);
|
||||
|
||||
byte[] data = new byte[width * height * 3];
|
||||
int index = 0;
|
||||
for (int y = 0; y < height; y++) {
|
||||
for (int x = 0; x < width; x++) {
|
||||
int rgb = image.getRGB(x, y);
|
||||
data[index++] = (byte) ((rgb >> 16) & 0xFF); // Red
|
||||
data[index++] = (byte) ((rgb >> 8) & 0xFF); // Green
|
||||
data[index++] = (byte) (rgb & 0xFF); // Blue
|
||||
}
|
||||
for (int rgb : pixels) {
|
||||
data[index++] = (byte) ((rgb >> 16) & 0xFF); // Red
|
||||
data[index++] = (byte) ((rgb >> 8) & 0xFF); // Green
|
||||
data[index++] = (byte) (rgb & 0xFF); // Blue
|
||||
}
|
||||
return data;
|
||||
}
|
||||
|
||||
@@ -32,83 +32,76 @@ public class InvertFullColorStrategy extends ReplaceAndInvertColorStrategy {
|
||||
|
||||
@Override
|
||||
public InputStreamResource replace() throws IOException {
|
||||
|
||||
File file = null;
|
||||
try {
|
||||
// Create a temporary file, with the original filename from the multipart file
|
||||
file = Files.createTempFile("temp", getFileInput().getOriginalFilename()).toFile();
|
||||
|
||||
try (TempFile tempFile =
|
||||
new TempFile(
|
||||
Files.createTempFile("temp", getFileInput().getOriginalFilename())
|
||||
.toFile())) {
|
||||
// Transfer the content of the multipart file to the file
|
||||
getFileInput().transferTo(file);
|
||||
getFileInput().transferTo(tempFile.getFile());
|
||||
|
||||
// Load the uploaded PDF
|
||||
PDDocument document = Loader.loadPDF(file);
|
||||
try (PDDocument document = Loader.loadPDF(tempFile.getFile())) {
|
||||
// Render each page and invert colors
|
||||
PDFRenderer pdfRenderer = new PDFRenderer(document);
|
||||
for (int page = 0; page < document.getNumberOfPages(); page++) {
|
||||
BufferedImage image;
|
||||
|
||||
// Render each page and invert colors
|
||||
PDFRenderer pdfRenderer = new PDFRenderer(document);
|
||||
for (int page = 0; page < document.getNumberOfPages(); page++) {
|
||||
BufferedImage image;
|
||||
// Use global maximum DPI setting, fallback to 300 if not set
|
||||
int renderDpi = 300; // Default fallback
|
||||
ApplicationProperties properties =
|
||||
ApplicationContextProvider.getBean(ApplicationProperties.class);
|
||||
if (properties != null && properties.getSystem() != null) {
|
||||
renderDpi = properties.getSystem().getMaxDPI();
|
||||
}
|
||||
final int dpi = renderDpi;
|
||||
final int pageNum = page;
|
||||
|
||||
// Use global maximum DPI setting, fallback to 300 if not set
|
||||
int renderDpi = 300; // Default fallback
|
||||
ApplicationProperties properties =
|
||||
ApplicationContextProvider.getBean(ApplicationProperties.class);
|
||||
if (properties != null && properties.getSystem() != null) {
|
||||
renderDpi = properties.getSystem().getMaxDPI();
|
||||
}
|
||||
final int dpi = renderDpi;
|
||||
final int pageNum = page;
|
||||
image =
|
||||
ExceptionUtils.handleOomRendering(
|
||||
pageNum + 1,
|
||||
dpi,
|
||||
() -> pdfRenderer.renderImageWithDPI(pageNum, dpi));
|
||||
|
||||
image =
|
||||
ExceptionUtils.handleOomRendering(
|
||||
pageNum + 1,
|
||||
dpi,
|
||||
() -> pdfRenderer.renderImageWithDPI(pageNum, dpi));
|
||||
// Invert the colors
|
||||
invertImageColors(image);
|
||||
|
||||
// Invert the colors
|
||||
invertImageColors(image);
|
||||
// Create a new PDPage from the inverted image
|
||||
PDPage pdPage = document.getPage(page);
|
||||
File tempImageFile = null;
|
||||
try {
|
||||
tempImageFile = convertToBufferedImageTpFile(image);
|
||||
PDImageXObject pdImage =
|
||||
PDImageXObject.createFromFileByContent(tempImageFile, document);
|
||||
|
||||
// Create a new PDPage from the inverted image
|
||||
PDPage pdPage = document.getPage(page);
|
||||
File tempImageFile = null;
|
||||
try {
|
||||
tempImageFile = convertToBufferedImageTpFile(image);
|
||||
PDImageXObject pdImage =
|
||||
PDImageXObject.createFromFileByContent(tempImageFile, document);
|
||||
|
||||
PDPageContentStream contentStream =
|
||||
new PDPageContentStream(
|
||||
document,
|
||||
pdPage,
|
||||
PDPageContentStream.AppendMode.OVERWRITE,
|
||||
true);
|
||||
contentStream.drawImage(
|
||||
pdImage,
|
||||
0,
|
||||
0,
|
||||
pdPage.getMediaBox().getWidth(),
|
||||
pdPage.getMediaBox().getHeight());
|
||||
contentStream.close();
|
||||
} finally {
|
||||
if (tempImageFile != null && tempImageFile.exists()) {
|
||||
Files.delete(tempImageFile.toPath());
|
||||
try (PDPageContentStream contentStream =
|
||||
new PDPageContentStream(
|
||||
document,
|
||||
pdPage,
|
||||
PDPageContentStream.AppendMode.OVERWRITE,
|
||||
true)) {
|
||||
contentStream.drawImage(
|
||||
pdImage,
|
||||
0,
|
||||
0,
|
||||
pdPage.getMediaBox().getWidth(),
|
||||
pdPage.getMediaBox().getHeight());
|
||||
}
|
||||
} finally {
|
||||
if (tempImageFile != null && tempImageFile.exists()) {
|
||||
Files.delete(tempImageFile.toPath());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Save the modified PDF to a ByteArrayOutputStream
|
||||
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
|
||||
document.save(byteArrayOutputStream);
|
||||
document.close();
|
||||
// Save the modified PDF to a ByteArrayOutputStream
|
||||
ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
|
||||
document.save(byteArrayOutputStream);
|
||||
|
||||
// Prepare the modified PDF for download
|
||||
ByteArrayInputStream inputStream =
|
||||
new ByteArrayInputStream(byteArrayOutputStream.toByteArray());
|
||||
InputStreamResource resource = new InputStreamResource(inputStream);
|
||||
return resource;
|
||||
} finally {
|
||||
if (file != null && file.exists()) {
|
||||
Files.delete(file.toPath());
|
||||
// Prepare the modified PDF for download
|
||||
ByteArrayInputStream inputStream =
|
||||
new ByteArrayInputStream(byteArrayOutputStream.toByteArray());
|
||||
InputStreamResource resource = new InputStreamResource(inputStream);
|
||||
return resource;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -117,18 +110,20 @@ public class InvertFullColorStrategy extends ReplaceAndInvertColorStrategy {
|
||||
private void invertImageColors(BufferedImage image) {
|
||||
int width = image.getWidth();
|
||||
int height = image.getHeight();
|
||||
for (int x = 0; x < width; x++) {
|
||||
for (int y = 0; y < height; y++) {
|
||||
int rgba = image.getRGB(x, y);
|
||||
Color color = new Color(rgba, true);
|
||||
Color invertedColor =
|
||||
new Color(
|
||||
255 - color.getRed(),
|
||||
255 - color.getGreen(),
|
||||
255 - color.getBlue());
|
||||
image.setRGB(x, y, invertedColor.getRGB());
|
||||
}
|
||||
int[] pixels = new int[width * height];
|
||||
image.getRGB(0, 0, width, height, pixels, 0, width);
|
||||
for (int i = 0; i < pixels.length; i++) {
|
||||
int pixel = pixels[i];
|
||||
|
||||
int a = 0xff;
|
||||
|
||||
int r = (pixel >> 16) & 0xff;
|
||||
int g = (pixel >> 8) & 0xff;
|
||||
int b = pixel & 0xff;
|
||||
|
||||
pixels[i] = (a << 24) | ((255 - r) << 16) | ((255 - g) << 8) | (255 - b);
|
||||
}
|
||||
image.setRGB(0, 0, width, height, pixels, 0, width);
|
||||
}
|
||||
|
||||
// Helper method to convert BufferedImage to InputStream
|
||||
@@ -137,4 +132,23 @@ public class InvertFullColorStrategy extends ReplaceAndInvertColorStrategy {
|
||||
ImageIO.write(image, "png", file);
|
||||
return file;
|
||||
}
|
||||
|
||||
private static class TempFile implements AutoCloseable {
|
||||
private final File file;
|
||||
|
||||
public TempFile(File file) {
|
||||
this.file = file;
|
||||
}
|
||||
|
||||
public File getFile() {
|
||||
return file;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void close() throws IOException {
|
||||
if (file != null && file.exists()) {
|
||||
Files.delete(file.toPath());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user