From 1a585bf1fa6ec29ce3e2f97b83185785c1e526c5 Mon Sep 17 00:00:00 2001 From: InAnYan Date: Mon, 16 Dec 2024 15:21:52 +0200 Subject: [PATCH 1/5] Fix some bugs with PDF imports --- .../UnlinkedFilesDialogViewModel.java | 1 - .../fileformat/PdfContentImporter.java | 20 ++++++++++--------- .../fileformat/PdfMergeMetadataImporter.java | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java b/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java index 58f634d0067..d4bab3f0bf1 100644 --- a/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java +++ b/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java @@ -149,7 +149,6 @@ public void startImport() { List fileList = checkedFileListProperty.stream() .map(item -> item.getValue().getPath()) .filter(path -> path.toFile().isFile()) - .map(path -> directory.relativize(path)) .collect(Collectors.toList()); if (fileList.isEmpty()) { LOGGER.warn("There are no valid files checked"); diff --git a/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java b/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java index 7d43be038a4..1339b217dd7 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java @@ -15,6 +15,8 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.swing.text.html.Option; + import org.jabref.logic.importer.ParserResult; import org.jabref.logic.l10n.Localization; import org.jabref.logic.os.OS; @@ -203,7 +205,7 @@ public ParserResult importDatabase(Path filePath) { List result = new ArrayList<>(1); try (PDDocument document = new XmpUtilReader().loadWithAutomaticDecryption(filePath)) { String firstPageContents = getFirstPageContents(document); - String titleByFontSize = extractTitleFromDocument(document); + Optional titleByFontSize = extractTitleFromDocument(document); Optional entry = getEntryFromPDFContent(firstPageContents, OS.NEWLINE, titleByFontSize); entry.ifPresent(result::add); } catch (EncryptedPdfsNotSupportedException e) { @@ -216,7 +218,7 @@ public ParserResult importDatabase(Path filePath) { return new ParserResult(result); } - private static String extractTitleFromDocument(PDDocument document) throws IOException { + private static Optional extractTitleFromDocument(PDDocument document) throws IOException { TitleExtractorByFontSize stripper = new TitleExtractorByFontSize(); return stripper.getTitle(document); } @@ -230,7 +232,7 @@ public TitleExtractorByFontSize() { this.textPositionsList = new ArrayList<>(); } - public String getTitle(PDDocument document) throws IOException { + public Optional getTitle(PDDocument document) throws IOException { this.setStartPage(1); this.setEndPage(2); this.writeText(document, new StringWriter()); @@ -266,7 +268,7 @@ private boolean isUnwantedText(TextPosition previousTextPosition, TextPosition t return isFarAway(previousTextPosition, textPosition); } - private String findLargestFontText(List textPositions) { + private Optional findLargestFontText(List textPositions) { Map fontSizeTextMap = new TreeMap<>(Collections.reverseOrder()); TextPosition previousTextPosition = null; for (TextPosition textPosition : textPositions) { @@ -285,10 +287,10 @@ private String findLargestFontText(List textPositions) { for (Map.Entry entry : fontSizeTextMap.entrySet()) { String candidateText = entry.getValue().toString().trim(); if (isLegalTitle(candidateText)) { - return candidateText; + return Optional.of(candidateText); } } - return fontSizeTextMap.values().iterator().next().toString().trim(); + return fontSizeTextMap.values().stream().findFirst().map(StringBuilder::toString).map(String::trim); } private boolean isLegalTitle(String candidateText) { @@ -334,7 +336,7 @@ private boolean isThereSpace(TextPosition previous, TextPosition current) { * is successful. Otherwise, an empty {@link Optional}. */ @VisibleForTesting - Optional getEntryFromPDFContent(String firstpageContents, String lineSeparator, String titleByFontSize) { + Optional getEntryFromPDFContent(String firstpageContents, String lineSeparator, Optional titleByFontSize) { String firstpageContentsUnifiedLineBreaks = StringUtil.unifyLineBreaks(firstpageContents, lineSeparator); lines = firstpageContentsUnifiedLineBreaks.split(lineSeparator); @@ -393,8 +395,8 @@ Optional getEntryFromPDFContent(String firstpageContents, String lineS title = streamlineTitle(curString); // i points to the next non-empty line curString = ""; - if (!isNullOrEmpty(titleByFontSize)) { - title = titleByFontSize; + if (titleByFontSize.isPresent() && !isNullOrEmpty(titleByFontSize.get())) { + title = titleByFontSize.get(); } // after title: authors diff --git a/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java b/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java index 21b18e27f84..0595c3b077d 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java @@ -150,7 +150,7 @@ public ParserResult importDatabase(Path filePath, BibDatabaseContext context, Fi List directories = context.getFileDirectories(filePreferences); - filePath = FileUtil.relativize(filePath, directories); + // filePath = FileUtil.relativize(filePath, directories); return importDatabase(filePath); } From a2ff5d15bc0ef2f7f2dd9a6994cf73b85dfde522 Mon Sep 17 00:00:00 2001 From: InAnYan Date: Mon, 16 Dec 2024 15:24:14 +0200 Subject: [PATCH 2/5] Fix checkstyle --- .../jabref/logic/importer/fileformat/PdfContentImporter.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java b/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java index 1339b217dd7..2c9c94a4745 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/PdfContentImporter.java @@ -15,8 +15,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import javax.swing.text.html.Option; - import org.jabref.logic.importer.ParserResult; import org.jabref.logic.l10n.Localization; import org.jabref.logic.os.OS; From 4a4c4051900c5443ea37d5f65ed6eb7f3cee146e Mon Sep 17 00:00:00 2001 From: InAnYan Date: Mon, 16 Dec 2024 15:33:47 +0200 Subject: [PATCH 3/5] Fix tests --- .../logic/importer/fileformat/PdfContentImporterTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jabref/logic/importer/fileformat/PdfContentImporterTest.java b/src/test/java/org/jabref/logic/importer/fileformat/PdfContentImporterTest.java index 318cb858508..43a551a8634 100644 --- a/src/test/java/org/jabref/logic/importer/fileformat/PdfContentImporterTest.java +++ b/src/test/java/org/jabref/logic/importer/fileformat/PdfContentImporterTest.java @@ -70,7 +70,7 @@ void parsingEditorWithoutPagesorSeriesInformation() { Corpus linguistics investigates human language by starting out from large """; - assertEquals(Optional.of(entry), importer.getEntryFromPDFContent(firstPageContents, "\n", "")); + assertEquals(Optional.of(entry), importer.getEntryFromPDFContent(firstPageContents, "\n", Optional.empty())); } @Test @@ -93,7 +93,7 @@ Smith, Lucy Anna (2014) Mortality in the Ornamental Fish Retail Sector: an Analy UNSPECIFIED Master of Research (MRes) thesis, University of Kent,."""; - assertEquals(Optional.of(entry), importer.getEntryFromPDFContent(firstPageContents, "\n", "")); + assertEquals(Optional.of(entry), importer.getEntryFromPDFContent(firstPageContents, "\n", Optional.empty())); } @Test @@ -126,7 +126,7 @@ British Journal of Nutrition (2008), 99, 1–11 doi: 10.1017/S0007114507795296 British Journal of Nutrition https://doi.org/10.1017/S0007114507795296 Published online by Cambridge University Press"""; - assertEquals(Optional.of(entry), importer.getEntryFromPDFContent(firstPageContent, "\n", "")); + assertEquals(Optional.of(entry), importer.getEntryFromPDFContent(firstPageContent, "\n", Optional.empty())); } @ParameterizedTest From becf8655ead66e4267b344afd7476ef89084f5c6 Mon Sep 17 00:00:00 2001 From: InAnYan Date: Wed, 18 Dec 2024 09:00:29 +0200 Subject: [PATCH 4/5] Update from code review --- .../UnlinkedFilesDialogViewModel.java | 51 ++++++++++--------- .../fileformat/PdfMergeMetadataImporter.java | 12 ----- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java b/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java index d4bab3f0bf1..50163b34515 100644 --- a/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java +++ b/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java @@ -10,7 +10,6 @@ import java.util.List; import java.util.Optional; import java.util.function.Predicate; -import java.util.stream.Collectors; import javax.swing.undo.UndoManager; @@ -145,29 +144,32 @@ public void startSearch() { } public void startImport() { - Path directory = this.getSearchDirectory(); - List fileList = checkedFileListProperty.stream() - .map(item -> item.getValue().getPath()) - .filter(path -> path.toFile().isFile()) - .collect(Collectors.toList()); + List fileList = checkedFileListProperty + .stream() + .map(TreeItem::getValue) + .map(FileNodeViewModel::getPath) + .filter(Files::isRegularFile) + .toList(); + if (fileList.isEmpty()) { - LOGGER.warn("There are no valid files checked"); + LOGGER.warn("There are no valid files checked for import"); return; } resultList.clear(); - importFilesBackgroundTask = importHandler.importFilesInBackground(fileList, bibDatabase, preferences.getFilePreferences(), TransferMode.LINK) - .onRunning(() -> { - progressValueProperty.bind(importFilesBackgroundTask.workDonePercentageProperty()); - progressTextProperty.bind(importFilesBackgroundTask.messageProperty()); - taskActiveProperty.setValue(true); - }) - .onFinished(() -> { - progressValueProperty.unbind(); - progressTextProperty.unbind(); - taskActiveProperty.setValue(false); - }) - .onSuccess(resultList::addAll); + importFilesBackgroundTask = importHandler + .importFilesInBackground(fileList, bibDatabase, preferences.getFilePreferences(), TransferMode.LINK) + .onRunning(() -> { + progressValueProperty.bind(importFilesBackgroundTask.workDonePercentageProperty()); + progressTextProperty.bind(importFilesBackgroundTask.messageProperty()); + taskActiveProperty.setValue(true); + }) + .onFinished(() -> { + progressValueProperty.unbind(); + progressTextProperty.unbind(); + taskActiveProperty.setValue(false); + }) + .onSuccess(resultList::addAll); importFilesBackgroundTask.executeWith(taskExecutor); } @@ -175,12 +177,13 @@ public void startImport() { * This starts the export of all files of all selected nodes in the file tree view. */ public void startExport() { - List fileList = checkedFileListProperty.stream() - .map(item -> item.getValue().getPath()) - .filter(path -> path.toFile().isFile()) - .toList(); + List fileList = checkedFileListProperty + .stream() + .map(item -> item.getValue().getPath()) + .filter(Files::isRegularFile) + .toList(); if (fileList.isEmpty()) { - LOGGER.warn("There are no valid files checked"); + LOGGER.warn("There are no valid files checked for export"); return; } diff --git a/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java b/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java index 0595c3b077d..bb461423a76 100644 --- a/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java +++ b/src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java @@ -136,22 +136,10 @@ public ParserResult importDatabase(Path filePath) throws IOException { return new ParserResult(List.of(entry)); } - /** - * A modified version of {@link PdfMergeMetadataImporter#importDatabase(Path)}, but it - * relativizes the {@code filePath} if there are working directories before parsing it - * into {@link PdfMergeMetadataImporter#importDatabase(Path)} - * (Otherwise no path modification happens). - * - * @param filePath The unrelativized {@code filePath}. - */ public ParserResult importDatabase(Path filePath, BibDatabaseContext context, FilePreferences filePreferences) throws IOException { Objects.requireNonNull(context); Objects.requireNonNull(filePreferences); - List directories = context.getFileDirectories(filePreferences); - - // filePath = FileUtil.relativize(filePath, directories); - return importDatabase(filePath); } From 15c9be610c1a4b823dec4d601692cce48985a0b4 Mon Sep 17 00:00:00 2001 From: InAnYan Date: Mon, 23 Dec 2024 15:21:06 +0200 Subject: [PATCH 5/5] Fix importing files --- src/main/java/org/jabref/gui/externalfiles/ImportHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java index 808764d0231..61c85b34d7f 100644 --- a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java +++ b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java @@ -147,7 +147,7 @@ public List call() { entry.clearField(StandardField.FILE); // Modifiers do not work on macOS: https://bugs.openjdk.org/browse/JDK-8264172 // Similar code as org.jabref.gui.preview.PreviewPanel.PreviewPanel - DragDrop.handleDropOfFiles(files, transferMode, fileLinker, entry); + DragDrop.handleDropOfFiles(List.of(file), transferMode, fileLinker, entry); entriesToAdd.addAll(pdfEntriesInFile); addResultToList(file, true, Localization.lang("File was successfully imported as a new entry")); });