From 3fdfb9632be7551b986e3067a8a67568880016ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Sz=C3=BCcs?= <127139797+balazs-szucs@users.noreply.github.com> Date: Mon, 17 Nov 2025 11:47:54 +0100 Subject: [PATCH] refactor(core): simplify resource management with try-with-resources (#4873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description of Changes TLDR: - Replaced manual resource handling with try-with-resources in multiple controllers - Eliminated redundant `finally` blocks for improved readability - Added missing resource closures to ensure proper cleanup - Updated `ScalePagesController` with static utility improvements and logging annotation This pull request focuses on improving resource management and code clarity across several PDF processing controllers. The main changes involve refactoring to use try-with-resources for automatic resource cleanup, removing manual close calls and finally blocks, and restructuring helper methods for better organization. These updates enhance code safety, maintainability, and performance. **Resource Management Improvements:** - Refactored multiple controllers (`EditTableOfContentsController`, `PdfOverlayController`, `ScalePagesController`, `SplitPDFController`, and `ScannerEffectController`) to use try-with-resources for managing `PDDocument`, `ByteArrayOutputStream`, and other closable resources, eliminating the need for manual close calls and finally blocks. This reduces the risk of resource leaks and simplifies the code. **Code Organization and Clarity:** - Moved and consolidated helper methods (`getTargetSize`, `getSizeMap`) in `ScalePagesController` for better encapsulation and static usage, and added the `@Slf4j` annotation for logging - Removed redundant imports and unused variables to clean up the codebase. **Performance Enhancements:** - Optimized object instantiation by creating reusable objects (e.g., `LayerUtility` in `ScalePagesController`) outside of loops to improve performance. --- ## 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) ### 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) - [X] 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 --- .../api/EditTableOfContentsController.java | 21 +-- .../controller/api/PdfOverlayController.java | 12 +- .../controller/api/ScalePagesController.java | 173 +++++++++++------- .../controller/api/SplitPDFController.java | 39 +--- .../api/misc/ScannerEffectController.java | 4 +- 5 files changed, 117 insertions(+), 132 deletions(-) diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTableOfContentsController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTableOfContentsController.java index cece9b20a..33b9ef067 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTableOfContentsController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/EditTableOfContentsController.java @@ -10,7 +10,6 @@ import org.apache.pdfbox.pdmodel.PDDocument; import org.apache.pdfbox.pdmodel.PDPage; import org.apache.pdfbox.pdmodel.interactive.documentnavigation.outline.PDDocumentOutline; import org.apache.pdfbox.pdmodel.interactive.documentnavigation.outline.PDOutlineItem; -import org.apache.pdfbox.pdmodel.interactive.documentnavigation.outline.PDOutlineNode; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.*; @@ -49,9 +48,7 @@ public class EditTableOfContentsController { @ResponseBody public List> extractBookmarks(@RequestParam("file") MultipartFile file) throws Exception { - PDDocument document = null; - try { - document = pdfDocumentFactory.load(file); + try (PDDocument document = pdfDocumentFactory.load(file)) { PDDocumentOutline outline = document.getDocumentCatalog().getDocumentOutline(); if (outline == null) { @@ -60,10 +57,6 @@ public class EditTableOfContentsController { } return extractBookmarkItems(document, outline); - } finally { - if (document != null) { - document.close(); - } } } @@ -92,7 +85,6 @@ public class EditTableOfContentsController { PDOutlineItem child = current.getFirstChild(); if (child != null) { List> children = new ArrayList<>(); - PDOutlineNode parent = current; while (child != null) { // Recursively process child items @@ -157,10 +149,9 @@ public class EditTableOfContentsController { public ResponseEntity editTableOfContents( @ModelAttribute EditTableOfContentsRequest request) throws Exception { MultipartFile file = request.getFileInput(); - PDDocument document = null; - try { - document = pdfDocumentFactory.load(file); + try (PDDocument document = pdfDocumentFactory.load(file); + ByteArrayOutputStream baos = new ByteArrayOutputStream()) { // Parse the bookmark data from JSON List bookmarks = @@ -175,18 +166,12 @@ public class EditTableOfContentsController { addBookmarksToOutline(document, outline, bookmarks); // Save the document to a byte array - ByteArrayOutputStream baos = new ByteArrayOutputStream(); document.save(baos); return WebResponseUtils.bytesToWebResponse( baos.toByteArray(), GeneralUtils.generateFilename(file.getOriginalFilename(), "_with_toc.pdf"), MediaType.APPLICATION_PDF); - - } finally { - if (document != null) { - document.close(); - } } } diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java index 2d987e420..3830fd65e 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/PdfOverlayController.java @@ -63,7 +63,8 @@ public class PdfOverlayController { int[] counts = request.getCounts(); // Used for FixedRepeatOverlay mode try (PDDocument basePdf = pdfDocumentFactory.load(baseFile); - Overlay overlay = new Overlay()) { + Overlay overlay = new Overlay(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { Map overlayGuide = prepareOverlayGuide( basePdf.getNumberOfPages(), @@ -79,7 +80,6 @@ public class PdfOverlayController { overlay.setOverlayPosition(Overlay.Position.BACKGROUND); } - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); overlay.overlay(overlayGuide).save(outputStream); byte[] data = outputStream.toByteArray(); String outputFilename = @@ -140,12 +140,11 @@ public class PdfOverlayController { overlayFileIndex = (overlayFileIndex + 1) % overlayFiles.length; } - try (PDDocument overlayPdf = Loader.loadPDF(overlayFiles[overlayFileIndex])) { - PDDocument singlePageDocument = new PDDocument(); + try (PDDocument overlayPdf = Loader.loadPDF(overlayFiles[overlayFileIndex]); + PDDocument singlePageDocument = new PDDocument()) { singlePageDocument.addPage(overlayPdf.getPage(pageCountInCurrentOverlay)); File tempFile = Files.createTempFile("overlay-page-", ".pdf").toFile(); singlePageDocument.save(tempFile); - singlePageDocument.close(); overlayGuide.put(basePageIndex, tempFile.getAbsolutePath()); tempFiles.add(tempFile); // Keep track of the temporary file for cleanup @@ -202,6 +201,3 @@ public class PdfOverlayController { } } } - -// Additional classes like OverlayPdfsRequest, WebResponseUtils, etc. are assumed to be defined -// elsewhere. diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/ScalePagesController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/ScalePagesController.java index e90aab8eb..85e94a7b4 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/ScalePagesController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/ScalePagesController.java @@ -24,6 +24,7 @@ import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.tags.Tag; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; import stirling.software.SPDF.model.api.general.ScalePagesRequest; import stirling.software.common.service.CustomPDFDocumentFactory; @@ -34,88 +35,22 @@ import stirling.software.common.util.WebResponseUtils; @RestController @RequestMapping("/api/v1/general") @Tag(name = "General", description = "General APIs") +@Slf4j @RequiredArgsConstructor public class ScalePagesController { private final CustomPDFDocumentFactory pdfDocumentFactory; - @PostMapping(value = "/scale-pages", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - @Operation( - summary = "Change the size of a PDF page/document", - description = - "This operation takes an input PDF file and the size to scale the pages to in" - + " the output PDF file. Input:PDF Output:PDF Type:SISO") - public ResponseEntity scalePages(@ModelAttribute ScalePagesRequest request) - throws IOException { - MultipartFile file = request.getFileInput(); - String targetPDRectangle = request.getPageSize(); - float scaleFactor = request.getScaleFactor(); - - PDDocument sourceDocument = pdfDocumentFactory.load(file); - PDDocument outputDocument = - pdfDocumentFactory.createNewDocumentBasedOnOldDocument(sourceDocument); - - PDRectangle targetSize = getTargetSize(targetPDRectangle, sourceDocument); - - int totalPages = sourceDocument.getNumberOfPages(); - for (int i = 0; i < totalPages; i++) { - PDPage sourcePage = sourceDocument.getPage(i); - PDRectangle sourceSize = sourcePage.getMediaBox(); - - float scaleWidth = targetSize.getWidth() / sourceSize.getWidth(); - float scaleHeight = targetSize.getHeight() / sourceSize.getHeight(); - float scale = Math.min(scaleWidth, scaleHeight) * scaleFactor; - - PDPage newPage = new PDPage(targetSize); - outputDocument.addPage(newPage); - - PDPageContentStream contentStream = - new PDPageContentStream( - outputDocument, - newPage, - PDPageContentStream.AppendMode.APPEND, - true, - true); - - float x = (targetSize.getWidth() - sourceSize.getWidth() * scale) / 2; - float y = (targetSize.getHeight() - sourceSize.getHeight() * scale) / 2; - - contentStream.saveGraphicsState(); - contentStream.transform(Matrix.getTranslateInstance(x, y)); - contentStream.transform(Matrix.getScaleInstance(scale, scale)); - - LayerUtility layerUtility = new LayerUtility(outputDocument); - PDFormXObject form = layerUtility.importPageAsForm(sourceDocument, i); - contentStream.drawForm(form); - - contentStream.restoreGraphicsState(); - contentStream.close(); - } - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - outputDocument.save(baos); - outputDocument.close(); - sourceDocument.close(); - - return WebResponseUtils.bytesToWebResponse( - baos.toByteArray(), - GeneralUtils.generateFilename(file.getOriginalFilename(), "_scaled.pdf")); - } - - private PDRectangle getTargetSize(String targetPDRectangle, PDDocument sourceDocument) { + private static PDRectangle getTargetSize(String targetPDRectangle, PDDocument sourceDocument) { if ("KEEP".equals(targetPDRectangle)) { if (sourceDocument.getNumberOfPages() == 0) { - // Do not return null here; throw a clear exception so callers don't get a nullable - // PDRectangle. throw ExceptionUtils.createInvalidPageSizeException("KEEP"); } - // use the first page to determine the target page size PDPage sourcePage = sourceDocument.getPage(0); PDRectangle sourceSize = sourcePage.getMediaBox(); if (sourceSize == null) { - // If media box is unexpectedly null, treat it as invalid throw ExceptionUtils.createInvalidPageSizeException("KEEP"); } @@ -131,9 +66,10 @@ public class ScalePagesController { throw ExceptionUtils.createInvalidPageSizeException(targetPDRectangle); } - private Map getSizeMap() { + private static Map getSizeMap() { Map sizeMap = new HashMap<>(); - // Add A0 - A6 + + // Portrait sizes (A0-A6) sizeMap.put("A0", PDRectangle.A0); sizeMap.put("A1", PDRectangle.A1); sizeMap.put("A2", PDRectangle.A2); @@ -142,10 +78,105 @@ public class ScalePagesController { sizeMap.put("A5", PDRectangle.A5); sizeMap.put("A6", PDRectangle.A6); - // Add other sizes + // Landscape sizes (A0-A6) + sizeMap.put( + "A0_LANDSCAPE", + new PDRectangle(PDRectangle.A0.getHeight(), PDRectangle.A0.getWidth())); + sizeMap.put( + "A1_LANDSCAPE", + new PDRectangle(PDRectangle.A1.getHeight(), PDRectangle.A1.getWidth())); + sizeMap.put( + "A2_LANDSCAPE", + new PDRectangle(PDRectangle.A2.getHeight(), PDRectangle.A2.getWidth())); + sizeMap.put( + "A3_LANDSCAPE", + new PDRectangle(PDRectangle.A3.getHeight(), PDRectangle.A3.getWidth())); + sizeMap.put( + "A4_LANDSCAPE", + new PDRectangle(PDRectangle.A4.getHeight(), PDRectangle.A4.getWidth())); + sizeMap.put( + "A5_LANDSCAPE", + new PDRectangle(PDRectangle.A5.getHeight(), PDRectangle.A5.getWidth())); + sizeMap.put( + "A6_LANDSCAPE", + new PDRectangle(PDRectangle.A6.getHeight(), PDRectangle.A6.getWidth())); + + // Portrait US sizes sizeMap.put("LETTER", PDRectangle.LETTER); sizeMap.put("LEGAL", PDRectangle.LEGAL); + // Landscape US sizes + sizeMap.put( + "LETTER_LANDSCAPE", + new PDRectangle(PDRectangle.LETTER.getHeight(), PDRectangle.LETTER.getWidth())); + sizeMap.put( + "LEGAL_LANDSCAPE", + new PDRectangle(PDRectangle.LEGAL.getHeight(), PDRectangle.LEGAL.getWidth())); + return sizeMap; } + + @PostMapping(value = "/scale-pages", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) + @Operation( + summary = "Change the size of a PDF page/document", + description = + "This operation takes an input PDF file and the size to scale the pages to in" + + " the output PDF file. Input:PDF Output:PDF Type:SISO") + public ResponseEntity scalePages(@ModelAttribute ScalePagesRequest request) + throws IOException { + MultipartFile file = request.getFileInput(); + String targetPDRectangle = request.getPageSize(); + float scaleFactor = request.getScaleFactor(); + + try (PDDocument sourceDocument = pdfDocumentFactory.load(file); + PDDocument outputDocument = + pdfDocumentFactory.createNewDocumentBasedOnOldDocument(sourceDocument); + ByteArrayOutputStream baos = new ByteArrayOutputStream()) { + + PDRectangle targetSize = getTargetSize(targetPDRectangle, sourceDocument); + + // Create LayerUtility once outside the loop for better performance + LayerUtility layerUtility = new LayerUtility(outputDocument); + + int totalPages = sourceDocument.getNumberOfPages(); + for (int i = 0; i < totalPages; i++) { + PDPage sourcePage = sourceDocument.getPage(i); + PDRectangle sourceSize = sourcePage.getMediaBox(); + + float scaleWidth = targetSize.getWidth() / sourceSize.getWidth(); + float scaleHeight = targetSize.getHeight() / sourceSize.getHeight(); + float scale = Math.min(scaleWidth, scaleHeight) * scaleFactor; + + PDPage newPage = new PDPage(targetSize); + outputDocument.addPage(newPage); + + try (PDPageContentStream contentStream = + new PDPageContentStream( + outputDocument, + newPage, + PDPageContentStream.AppendMode.APPEND, + true, + true)) { + + float x = (targetSize.getWidth() - sourceSize.getWidth() * scale) / 2; + float y = (targetSize.getHeight() - sourceSize.getHeight() * scale) / 2; + + contentStream.saveGraphicsState(); + contentStream.transform(Matrix.getTranslateInstance(x, y)); + contentStream.transform(Matrix.getScaleInstance(scale, scale)); + + PDFormXObject form = layerUtility.importPageAsForm(sourceDocument, i); + contentStream.drawForm(form); + + contentStream.restoreGraphicsState(); + } + } + + outputDocument.save(baos); + + return WebResponseUtils.bytesToWebResponse( + baos.toByteArray(), + GeneralUtils.generateFilename(file.getOriginalFilename(), "_scaled.pdf")); + } + } } diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java index 0c1def9f1..8a962bd8a 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/SplitPDFController.java @@ -54,15 +54,12 @@ public class SplitPDFController { public ResponseEntity splitPdf(@ModelAttribute PDFWithPageNums request) throws IOException { - PDDocument document = null; - List splitDocumentsBoas = new ArrayList<>(); - TempFile outputTempFile = null; + MultipartFile file = request.getFileInput(); - try { - outputTempFile = new TempFile(tempFileManager, ".zip"); + try (TempFile outputTempFile = new TempFile(tempFileManager, ".zip"); + PDDocument document = pdfDocumentFactory.load(file)) { - MultipartFile file = request.getFileInput(); - document = pdfDocumentFactory.load(file); + List splitDocumentsBoas = new ArrayList<>(); int totalPages = document.getNumberOfPages(); List pageNumbers = request.getPageNumbersList(document, false); @@ -80,7 +77,8 @@ public class SplitPDFController { int previousPageNumber = 0; for (int splitPoint : pageNumbers) { try (PDDocument splitDocument = - pdfDocumentFactory.createNewDocumentBasedOnOldDocument(document)) { + pdfDocumentFactory.createNewDocumentBasedOnOldDocument(document); + ByteArrayOutputStream baos = new ByteArrayOutputStream()) { for (int i = previousPageNumber; i <= splitPoint; i++) { PDPage page = document.getPage(i); splitDocument.addPage(page); @@ -91,7 +89,6 @@ public class SplitPDFController { // Transfer metadata to split pdf // PdfMetadataService.setMetadataToPdf(splitDocument, metadata); - ByteArrayOutputStream baos = new ByteArrayOutputStream(); splitDocument.save(baos); splitDocumentsBoas.add(baos); } catch (Exception e) { @@ -100,8 +97,6 @@ public class SplitPDFController { } } - document.close(); - String baseFilename = GeneralUtils.removeExtension(file.getOriginalFilename()); try (ZipOutputStream zipOut = @@ -133,28 +128,6 @@ public class SplitPDFController { GeneralUtils.generateFilename(file.getOriginalFilename(), "_split.zip"); return WebResponseUtils.bytesToWebResponse( data, zipFilename, MediaType.APPLICATION_OCTET_STREAM); - - } finally { - try { - // Close the main document - if (document != null) { - document.close(); - } - - // Close all ByteArrayOutputStreams - for (ByteArrayOutputStream baos : splitDocumentsBoas) { - if (baos != null) { - baos.close(); - } - } - - // Close the output temporary file - if (outputTempFile != null) { - outputTempFile.close(); - } - } catch (Exception e) { - log.error("Error while cleaning up resources", e); - } } } } diff --git a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ScannerEffectController.java b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ScannerEffectController.java index 41a558e50..05c5b6ac7 100644 --- a/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ScannerEffectController.java +++ b/app/core/src/main/java/stirling/software/SPDF/controller/api/misc/ScannerEffectController.java @@ -621,7 +621,8 @@ public class ScannerEffectController { sharedPdfBytes != null ? pdfDocumentFactory.load(sharedPdfBytes) : pdfDocumentFactory.load(processingInput); - PDDocument outputDocument = new PDDocument()) { + PDDocument outputDocument = new PDDocument(); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { int totalPages = document.getNumberOfPages(); if (totalPages == 0) { @@ -701,7 +702,6 @@ public class ScannerEffectController { writeProcessedPagesToDocument(processedPages, outputDocument); - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); outputDocument.save(outputStream); return WebResponseUtils.bytesToWebResponse(