From 6ae4d16bb7e57fa104b0206a5857ae3c676627e9 Mon Sep 17 00:00:00 2001 From: Viktor Seifert Date: Fri, 24 Feb 2023 13:32:27 +0100 Subject: [PATCH] RED-6204: Corrected temp file deletion in PdfSegmentationService. Previously files were not deleted correctly, because the code tried to delete the file while a file-stream was still open. --- .../segmentation/PdfSegmentationService.java | 99 ++++++++++--------- .../tableextraction/utils/FileUtils.java | 23 ++++- 2 files changed, 70 insertions(+), 52 deletions(-) diff --git a/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/segmentation/PdfSegmentationService.java b/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/segmentation/PdfSegmentationService.java index be16069b..8223b17f 100644 --- a/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/segmentation/PdfSegmentationService.java +++ b/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/segmentation/PdfSegmentationService.java @@ -53,15 +53,14 @@ public class PdfSegmentationService { public Document parseDocument(String dossierId, String fileId, InputStream documentInputStream, Map> pdfImages) throws IOException { PDDocument pdDocument = null; + File tempFile = null; try { - //create tempFile - File tempFile = FileUtils.createTempFile("document", ".pdf"); - Map> pdfTableCells = new HashMap<>(); if (redactionServiceSettings.isCvTableParsingEnabled()) { pdfTableCells = tableService.convertTables(dossierId, fileId); } + tempFile = FileUtils.createTempFile("document", ".pdf"); try (var fos = new FileOutputStream(tempFile)) { IOUtils.copy(documentInputStream, fos); @@ -74,44 +73,7 @@ public class PdfSegmentationService { long pageCount = pdDocument.getNumberOfPages(); for (int pageNumber = 1; pageNumber <= pageCount; pageNumber++) { - - PDFLinesTextStripper stripper = new PDFLinesTextStripper(); - PDPage pdPage = pdDocument.getPage(pageNumber - 1); - stripper.setPageNumber(pageNumber); - stripper.setStartPage(pageNumber); - stripper.setEndPage(pageNumber); - stripper.setPdpage(pdPage); - stripper.getText(pdDocument); - - PDRectangle pdr = pdPage.getMediaBox(); - - int rotation = pdPage.getRotation(); - boolean isLandscape = pdr.getWidth() > pdr.getHeight() && (rotation == 0 || rotation == 180) || pdr.getHeight() > pdr.getWidth() && (rotation == 90 || rotation == 270); - - PDRectangle cropbox = pdPage.getCropBox(); - CleanRulings cleanRulings = rulingCleaningService.getCleanRulings(pdfTableCells.get(pageNumber), - stripper.getRulings(), - stripper.getMinCharWidth(), - stripper.getMaxCharHeight()); - Page page = blockificationService.blockify(stripper.getTextPositionSequences(), cleanRulings.getHorizontal(), cleanRulings.getVertical()); - - page.setRotation(rotation); - page.setLandscape(isLandscape); - page.setPageNumber(pageNumber); - page.setPageWidth(cropbox.getWidth()); - page.setPageHeight(cropbox.getHeight()); - - // If images is ocr needs to be calculated before textBlocks are moved into tables, otherwise findOcr algorithm needs to be adopted. - if (pdfImages != null && pdfImages.containsKey(pageNumber)) { - page.setImages(pdfImages.get(pageNumber)); - imageService.findOcr(page); - } - - tableExtractionService.extractTables(cleanRulings, page); - buildPageStatistics(page); - increaseDocumentStatistics(page, document); - - pages.add(page); + processPage(pdfImages, pdDocument, pdfTableCells, document, pages, pageNumber); } document.setPages(pages); @@ -120,22 +82,65 @@ public class PdfSegmentationService { sectionsBuilderService.buildSections(document); sectionsBuilderService.addImagesToSections(document); - IOUtils.close(pdDocument); - - if (!tempFile.delete()) { - log.warn("Could not delete tmp file"); - } - return document; } } finally { if (pdDocument != null) { pdDocument.close(); } + + FileUtils.deleteFile(tempFile); } } + private void processPage(Map> pdfImages, + PDDocument pdDocument, + Map> pdfTableCells, + Document document, + List pages, + int pageNumber) throws IOException { + + PDFLinesTextStripper stripper = new PDFLinesTextStripper(); + PDPage pdPage = pdDocument.getPage(pageNumber - 1); + stripper.setPageNumber(pageNumber); + stripper.setStartPage(pageNumber); + stripper.setEndPage(pageNumber); + stripper.setPdpage(pdPage); + stripper.getText(pdDocument); + + PDRectangle pdr = pdPage.getMediaBox(); + + int rotation = pdPage.getRotation(); + boolean isLandscape = pdr.getWidth() > pdr.getHeight() && (rotation == 0 || rotation == 180) || pdr.getHeight() > pdr.getWidth() && (rotation == 90 || rotation == 270); + + PDRectangle cropbox = pdPage.getCropBox(); + CleanRulings cleanRulings = rulingCleaningService.getCleanRulings(pdfTableCells.get(pageNumber), + stripper.getRulings(), + stripper.getMinCharWidth(), + stripper.getMaxCharHeight()); + Page page = blockificationService.blockify(stripper.getTextPositionSequences(), cleanRulings.getHorizontal(), cleanRulings.getVertical()); + + page.setRotation(rotation); + page.setLandscape(isLandscape); + page.setPageNumber(pageNumber); + page.setPageWidth(cropbox.getWidth()); + page.setPageHeight(cropbox.getHeight()); + + // If images is ocr needs to be calculated before textBlocks are moved into tables, otherwise findOcr algorithm needs to be adopted. + if (pdfImages != null && pdfImages.containsKey(pageNumber)) { + page.setImages(pdfImages.get(pageNumber)); + imageService.findOcr(page); + } + + tableExtractionService.extractTables(cleanRulings, page); + buildPageStatistics(page); + increaseDocumentStatistics(page, document); + + pages.add(page); + } + + private void increaseDocumentStatistics(Page page, Document document) { if (!page.isLandscape()) { diff --git a/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/tableextraction/utils/FileUtils.java b/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/tableextraction/utils/FileUtils.java index aa8cd148..59919fa9 100644 --- a/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/tableextraction/utils/FileUtils.java +++ b/redaction-service-v1/redaction-service-server-v1/src/main/java/com/iqser/red/service/redaction/v1/server/tableextraction/utils/FileUtils.java @@ -3,6 +3,7 @@ package com.iqser.red.service.redaction.v1.server.tableextraction.utils; import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.nio.file.Path; import lombok.experimental.UtilityClass; import lombok.extern.slf4j.Slf4j; @@ -20,19 +21,31 @@ public class FileUtils { } + /** + * Deletes a files; logs a message with the reason if the deletions fails. + * This method is null-safe. + * @param file The file to delete. Can be null. + */ + public void deleteFile(File file) { + if (file != null) { + try { + Files.deleteIfExists(Path.of(file.toString())); + } catch (IOException ex) { + log.warn("Could not delete file!", ex); + } + } + } + + // We don't need to check the results of the permission setters below, // since we're manipulating a file we created ourselves. @SuppressWarnings("ResultOfMethodCallIgnored") private void setRWPermissionsOnlyForOwner(File tempFile) { try { - // deny for all - tempFile.setReadable(false); - tempFile.setWritable(false); - tempFile.setExecutable(false); - // allow for owner tempFile.setReadable(true, true); tempFile.setWritable(true, true); + tempFile.setExecutable(false); } catch (SecurityException ex) { // This should never happen since we're creating a temp file ourselves. log.warn("Caught an exception during temp file creation. This should not happend. Check the code.", ex);