From 133a3249bafefad4a543e6cc12c04f4c14aa00cb Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Tue, 8 Jul 2025 13:23:03 +0530 Subject: [PATCH 01/24] refactor: split OpenExternalFileAction into OpenSingleExternalFileAction and OpenSelectedEntriesFilesAction - Introduced OpenSingleExternalFileAction to handle single entry with one linked file - Introduced OpenSelectedEntriesFilesAction to handle multiple selection cases Fixes #13431 --- .../org/jabref/gui/actions/ActionHelper.java | 6 +- .../jabref/gui/actions/StandardActions.java | 2 + .../FulltextSearchResultsTab.java | 5 +- .../org/jabref/gui/maintable/MainTable.java | 6 +- .../gui/maintable/OpenExternalFileAction.java | 109 ------------------ .../OpenSelectedEntriesFilesAction.java | 92 +++++++++++++++ .../OpenSingleExternalFileAction.java | 48 ++++++++ .../jabref/gui/maintable/RightClickMenu.java | 2 +- 8 files changed, 151 insertions(+), 119 deletions(-) delete mode 100644 jabgui/src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java create mode 100644 jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java create mode 100644 jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java diff --git a/jabgui/src/main/java/org/jabref/gui/actions/ActionHelper.java b/jabgui/src/main/java/org/jabref/gui/actions/ActionHelper.java index f862ec08069..68a336b8bc4 100644 --- a/jabgui/src/main/java/org/jabref/gui/actions/ActionHelper.java +++ b/jabgui/src/main/java/org/jabref/gui/actions/ActionHelper.java @@ -64,8 +64,8 @@ public static BooleanExpression isAnyFieldSetForSelectedEntry(List fields ObservableList selectedEntries = stateManager.getSelectedEntries(); Binding fieldsAreSet = EasyBind.valueAt(selectedEntries, 0) .mapObservable(entry -> Bindings.createBooleanBinding( - () -> entry.getFields().stream().anyMatch(fields::contains), - entry.getFieldsObservable())) + () -> entry.getFields().stream().anyMatch(fields::contains), + entry.getFieldsObservable())) .orElseOpt(false); return BooleanExpression.booleanExpression(fieldsAreSet); } @@ -96,7 +96,7 @@ public static BooleanExpression isFilePresentForSelectedEntry(StateManager state /** * Check if at least one of the selected entries has linked files *
- * Used in {@link org.jabref.gui.maintable.OpenExternalFileAction} when multiple entries selected + * Used in {@link org.jabref.gui.maintable.OpenSelectedEntriesFilesAction} when multiple entries selected * * @param stateManager manager for the state of the GUI * @return a boolean binding diff --git a/jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java b/jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java index 446511c7857..f21144c1d0a 100644 --- a/jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java +++ b/jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java @@ -185,6 +185,8 @@ public enum StandardActions implements Action { OPEN_DEV_VERSION_LINK(Localization.lang("Development version"), Localization.lang("Opens a link where the current development version can be downloaded")), OPEN_CHANGELOG(Localization.lang("View change log"), Localization.lang("See what has been changed in the JabRef versions")), OPEN_GITHUB("GitHub", Localization.lang("Opens JabRef's GitHub page"), IconTheme.JabRefIcons.GITHUB), + WALKTHROUGH_MENU(Localization.lang("Walkthroughs"), IconTheme.JabRefIcons.BOOK), + MAIN_FILE_DIRECTORY_WALKTHROUGH(Localization.lang("Configure main file directory"), IconTheme.JabRefIcons.LATEX_FILE_DIRECTORY), DONATE(Localization.lang("Donate to JabRef"), Localization.lang("Donate to JabRef"), IconTheme.JabRefIcons.DONATE), OPEN_FORUM(Localization.lang("Community forum"), Localization.lang("Community forum"), IconTheme.JabRefIcons.FORUM), ERROR_CONSOLE(Localization.lang("View event log"), Localization.lang("Display all error messages")), diff --git a/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java b/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java index 1331c30bc6c..3b35ada8da2 100644 --- a/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java +++ b/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java @@ -24,8 +24,8 @@ import org.jabref.gui.documentviewer.DocumentViewerView; import org.jabref.gui.entryeditor.EntryEditor; import org.jabref.gui.entryeditor.EntryEditorTab; -import org.jabref.gui.maintable.OpenExternalFileAction; import org.jabref.gui.maintable.OpenFolderAction; +import org.jabref.gui.maintable.OpenSingleExternalFileAction; import org.jabref.gui.preferences.GuiPreferences; import org.jabref.gui.search.SearchType; import org.jabref.gui.util.TooltipTextUtil; @@ -180,8 +180,9 @@ private ContextMenu getFileContextMenu(LinkedFile file) { ContextMenu fileContextMenu = new ContextMenu(); fileContextMenu.getItems().add(actionFactory.createMenuItem( StandardActions.OPEN_FOLDER, new OpenFolderAction(dialogService, stateManager, preferences, entry, file, taskExecutor))); + BibDatabaseContext databaseContext = stateManager.getActiveDatabase().get(); fileContextMenu.getItems().add(actionFactory.createMenuItem( - StandardActions.OPEN_EXTERNAL_FILE, new OpenExternalFileAction(dialogService, stateManager, preferences, entry, file, taskExecutor))); + StandardActions.OPEN_EXTERNAL_FILE, new OpenSingleExternalFileAction(dialogService, preferences, entry, file, taskExecutor, databaseContext))); return fileContextMenu; } diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java b/jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java index 62a840904d6..982854f14c0 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/MainTable.java @@ -75,7 +75,6 @@ @AllowedToUseClassGetResource("JavaFX internally handles the passed URLs properly.") public class MainTable extends TableView { - private static final Logger LOGGER = LoggerFactory.getLogger(MainTable.class); private static final PseudoClass MATCHING_SEARCH_AND_GROUPS = PseudoClass.getPseudoClass("matching-search-and-groups"); private static final PseudoClass MATCHING_SEARCH_NOT_GROUPS = PseudoClass.getPseudoClass("matching-search-not-groups"); @@ -112,7 +111,6 @@ public MainTable(MainTableDataModel model, TaskExecutor taskExecutor, ImportHandler importHandler) { super(); - this.libraryTab = libraryTab; this.stateManager = stateManager; this.database = Objects.requireNonNull(database); @@ -369,7 +367,7 @@ private void setupKeyBindings(KeyBindingRepository keyBindings) { EditAction cutAction = new EditAction(StandardActions.CUT, () -> libraryTab, stateManager, undoManager); EditAction deleteAction = new EditAction(StandardActions.DELETE_ENTRY, () -> libraryTab, stateManager, undoManager); OpenUrlAction openUrlAction = new OpenUrlAction(dialogService, stateManager, preferences); - OpenExternalFileAction openExternalFileActionFileAction = new OpenExternalFileAction(dialogService, stateManager, preferences, taskExecutor); + OpenSelectedEntriesFilesAction openSelectedEntriesFilesActionFileAction = new OpenSelectedEntriesFilesAction(dialogService, stateManager, preferences, taskExecutor); MergeWithFetchedEntryAction mergeWithFetchedEntryAction = new MergeWithFetchedEntryAction(dialogService, stateManager, taskExecutor, preferences, undoManager); LookupIdentifierAction lookupIdentifierAction = new LookupIdentifierAction<>(WebFetchers.getIdFetcherForIdentifier(DOI.class), stateManager, undoManager, dialogService, taskExecutor); @@ -422,7 +420,7 @@ private void setupKeyBindings(KeyBindingRepository keyBindings) { event.consume(); break; case OPEN_FILE: - openExternalFileActionFileAction.execute(); + openSelectedEntriesFilesActionFileAction.execute(); event.consume(); break; case MERGE_WITH_FETCHED_ENTRY: diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java deleted file mode 100644 index 01265c3a1d0..00000000000 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenExternalFileAction.java +++ /dev/null @@ -1,109 +0,0 @@ -package org.jabref.gui.maintable; - -import java.util.LinkedList; -import java.util.List; - -import org.jabref.gui.DialogService; -import org.jabref.gui.StateManager; -import org.jabref.gui.actions.ActionHelper; -import org.jabref.gui.actions.SimpleCommand; -import org.jabref.gui.fieldeditors.LinkedFileViewModel; -import org.jabref.gui.preferences.GuiPreferences; -import org.jabref.logic.l10n.Localization; -import org.jabref.logic.util.TaskExecutor; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.LinkedFile; - -public class OpenExternalFileAction extends SimpleCommand { - - private final int FILES_LIMIT = 10; - - private final DialogService dialogService; - private final StateManager stateManager; - private final GuiPreferences preferences; - - private final BibEntry entry; - private final LinkedFile linkedFile; - private final TaskExecutor taskExecutor; - - public OpenExternalFileAction(DialogService dialogService, - StateManager stateManager, - GuiPreferences preferences, - TaskExecutor taskExecutor) { - this(dialogService, stateManager, preferences, null, null, taskExecutor); - } - - public OpenExternalFileAction(DialogService dialogService, - StateManager stateManager, - GuiPreferences preferences, - BibEntry entry, - LinkedFile linkedFile, - TaskExecutor taskExecutor) { - this.dialogService = dialogService; - this.stateManager = stateManager; - this.preferences = preferences; - this.entry = entry; - this.linkedFile = linkedFile; - this.taskExecutor = taskExecutor; - - if (this.linkedFile == null) { - this.executable.bind(ActionHelper.hasLinkedFileForSelectedEntries(stateManager) - .and(ActionHelper.needsEntriesSelected(stateManager)) - ); - } else { - this.setExecutable(true); - } - } - - /** - * Open all linked files of the selected entries. If opening too many files, pop out a dialog to ask the user if to continue. - *
- * If some selected entries have linked file and others do not, ignore the latter. - */ - @Override - public void execute() { - stateManager.getActiveDatabase().ifPresent(databaseContext -> { - if (entry == null) { - final List selectedEntries = stateManager.getSelectedEntries(); - - List linkedFileViewModelList = new LinkedList<>(); - LinkedFileViewModel linkedFileViewModel; - - for (BibEntry entry : selectedEntries) { - for (LinkedFile linkedFile : entry.getFiles()) { - linkedFileViewModel = new LinkedFileViewModel( - linkedFile, - entry, - databaseContext, - taskExecutor, - dialogService, - preferences); - - linkedFileViewModelList.add(linkedFileViewModel); - } - } - - // ask the user when detecting # of files > FILES_LIMIT - if (linkedFileViewModelList.size() > FILES_LIMIT) { - boolean continueOpening = dialogService.showConfirmationDialogAndWait(Localization.lang("Opening large number of files"), - Localization.lang("You are about to open %0 files. Continue?", linkedFileViewModelList.size()), - Localization.lang("Continue"), Localization.lang("Cancel")); - if (!continueOpening) { - return; - } - } - - linkedFileViewModelList.forEach(LinkedFileViewModel::open); - } else { - LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel( - linkedFile, - entry, - databaseContext, - taskExecutor, - dialogService, - preferences); - linkedFileViewModel.open(); - } - }); - } -} diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java new file mode 100644 index 00000000000..668f69f729c --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java @@ -0,0 +1,92 @@ +package org.jabref.gui.maintable; + +import java.util.LinkedList; +import java.util.List; + +import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; +import org.jabref.gui.actions.ActionHelper; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.fieldeditors.LinkedFileViewModel; +import org.jabref.gui.preferences.GuiPreferences; +import org.jabref.logic.l10n.Localization; +import org.jabref.logic.util.TaskExecutor; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.LinkedFile; + +public class OpenSelectedEntriesFilesAction extends SimpleCommand { + + private final int FILES_LIMIT = 10; + + private final DialogService dialogService; + private final StateManager stateManager; + private final GuiPreferences preferences; + private final TaskExecutor taskExecutor; + + public OpenSelectedEntriesFilesAction(DialogService dialogService, + StateManager stateManager, + GuiPreferences preferences, + TaskExecutor taskExecutor) { + this.dialogService = dialogService; + this.stateManager = stateManager; + this.preferences = preferences; + this.taskExecutor = taskExecutor; + + this.executable.bind(ActionHelper.hasLinkedFileForSelectedEntries(stateManager) + .and(ActionHelper.needsEntriesSelected(stateManager))); + } + + @Override + public void execute() { + stateManager.getActiveDatabase().ifPresent(databaseContext -> { + final List selectedEntries = stateManager.getSelectedEntries(); + + if (selectedEntries.size() == 1) { + BibEntry entry = selectedEntries.getFirst(); + List files = entry.getFiles(); + + if (files.size() == 1) { + new OpenSingleExternalFileAction( + dialogService, + preferences, + entry, + files.getFirst(), + taskExecutor, + databaseContext + ).execute(); + return; + } + } + + List linkedFileViewModelList = new LinkedList<>(); + + for (BibEntry entry : selectedEntries) { + for (LinkedFile linkedFile : entry.getFiles()) { + LinkedFileViewModel viewModel = new LinkedFileViewModel( + linkedFile, + entry, + databaseContext, + taskExecutor, + dialogService, + preferences); + + linkedFileViewModelList.add(viewModel); + } + } + + if (linkedFileViewModelList.size() > FILES_LIMIT) { + boolean continueOpening = dialogService.showConfirmationDialogAndWait( + Localization.lang("Opening large number of files"), + Localization.lang("You are about to open %0 files. Continue?", linkedFileViewModelList.size()), + Localization.lang("Continue"), + Localization.lang("Cancel") + ); + if (!continueOpening) { + return; + } + } + + linkedFileViewModelList.forEach(LinkedFileViewModel::open); + }); + } +} diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java new file mode 100644 index 00000000000..ad9a589b737 --- /dev/null +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java @@ -0,0 +1,48 @@ +package org.jabref.gui.maintable; + +import org.jabref.gui.DialogService; +import org.jabref.gui.actions.SimpleCommand; +import org.jabref.gui.fieldeditors.LinkedFileViewModel; +import org.jabref.gui.preferences.GuiPreferences; +import org.jabref.logic.util.TaskExecutor; +import org.jabref.model.database.BibDatabaseContext; +import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.LinkedFile; + +public class OpenSingleExternalFileAction extends SimpleCommand { + + private final DialogService dialogService; + private final GuiPreferences preferences; + private final BibEntry entry; + private final LinkedFile linkedFile; + private final TaskExecutor taskExecutor; + private final BibDatabaseContext databaseContext; + + public OpenSingleExternalFileAction(DialogService dialogService, + GuiPreferences preferences, + BibEntry entry, + LinkedFile linkedFile, + TaskExecutor taskExecutor, + BibDatabaseContext databaseContext) { + this.dialogService = dialogService; + this.preferences = preferences; + this.entry = entry; + this.linkedFile = linkedFile; + this.taskExecutor = taskExecutor; + this.databaseContext = databaseContext; + + this.setExecutable(true); + } + + @Override + public void execute() { + LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel( + linkedFile, + entry, + databaseContext, // gets database context from entry + taskExecutor, + dialogService, + preferences); + linkedFileViewModel.open(); + } +} diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/RightClickMenu.java b/jabgui/src/main/java/org/jabref/gui/maintable/RightClickMenu.java index 5a9c93e6cac..ba4d8ab2119 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/RightClickMenu.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/RightClickMenu.java @@ -96,7 +96,7 @@ public static ContextMenu create(BibEntryTableViewModel entry, factory.createMenuItem(StandardActions.ATTACH_FILE, new AttachFileAction(libraryTab, dialogService, stateManager, preferences.getFilePreferences(), preferences.getExternalApplicationsPreferences())), factory.createMenuItem(StandardActions.ATTACH_FILE_FROM_URL, new AttachFileFromURLAction(dialogService, stateManager, taskExecutor, preferences)), factory.createMenuItem(StandardActions.OPEN_FOLDER, new OpenFolderAction(dialogService, stateManager, preferences, taskExecutor)), - factory.createMenuItem(StandardActions.OPEN_EXTERNAL_FILE, new OpenExternalFileAction(dialogService, stateManager, preferences, taskExecutor)), + factory.createMenuItem(StandardActions.OPEN_EXTERNAL_FILE, new OpenSelectedEntriesFilesAction(dialogService, stateManager, preferences, taskExecutor)), extractFileReferencesOnline, extractFileReferencesOffline, From bcf3b76ebd33a38bc1f69b974ffadcb56d09d1b3 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Tue, 8 Jul 2025 14:16:54 +0530 Subject: [PATCH 02/24] Cleaned up constant name, dialog text, and removed extra comment --- .../gui/maintable/OpenSelectedEntriesFilesAction.java | 6 +++--- .../jabref/gui/maintable/OpenSingleExternalFileAction.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java index 668f69f729c..18cb719306b 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java @@ -16,7 +16,7 @@ public class OpenSelectedEntriesFilesAction extends SimpleCommand { - private final int FILES_LIMIT = 10; + private static final int FILES_LIMIT = 10; private final DialogService dialogService; private final StateManager stateManager; @@ -78,8 +78,8 @@ public void execute() { boolean continueOpening = dialogService.showConfirmationDialogAndWait( Localization.lang("Opening large number of files"), Localization.lang("You are about to open %0 files. Continue?", linkedFileViewModelList.size()), - Localization.lang("Continue"), - Localization.lang("Cancel") + Localization.lang("Open all files"), + Localization.lang("Don't open") ); if (!continueOpening) { return; diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java index ad9a589b737..cae7a1987db 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java @@ -39,7 +39,7 @@ public void execute() { LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel( linkedFile, entry, - databaseContext, // gets database context from entry + databaseContext, taskExecutor, dialogService, preferences); From 6ccf927e4a01fbe709b186d225af9cde74a78096 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Tue, 8 Jul 2025 14:32:12 +0530 Subject: [PATCH 03/24] Add null checks to OpenSingleExternalFileAction constructor --- .../maintable/OpenSingleExternalFileAction.java | 14 ++++++++------ jablib/src/main/abbrv.jabref.org | 2 +- jablib/src/main/resources/csl-locales | 2 +- jablib/src/main/resources/csl-styles | 2 +- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java index cae7a1987db..7721847a9ea 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java @@ -1,5 +1,7 @@ package org.jabref.gui.maintable; +import java.util.Objects; + import org.jabref.gui.DialogService; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.fieldeditors.LinkedFileViewModel; @@ -24,12 +26,12 @@ public OpenSingleExternalFileAction(DialogService dialogService, LinkedFile linkedFile, TaskExecutor taskExecutor, BibDatabaseContext databaseContext) { - this.dialogService = dialogService; - this.preferences = preferences; - this.entry = entry; - this.linkedFile = linkedFile; - this.taskExecutor = taskExecutor; - this.databaseContext = databaseContext; + this.dialogService = Objects.requireNonNull(dialogService); + this.preferences = Objects.requireNonNull(preferences); + this.entry = Objects.requireNonNull(entry); + this.linkedFile = Objects.requireNonNull(linkedFile); + this.taskExecutor = Objects.requireNonNull(taskExecutor); + this.databaseContext = Objects.requireNonNull(databaseContext); this.setExecutable(true); } diff --git a/jablib/src/main/abbrv.jabref.org b/jablib/src/main/abbrv.jabref.org index 193b23f48f1..333c2f19f96 160000 --- a/jablib/src/main/abbrv.jabref.org +++ b/jablib/src/main/abbrv.jabref.org @@ -1 +1 @@ -Subproject commit 193b23f48f1f137fe849781c2ecab6d32e27a86d +Subproject commit 333c2f19f96b760f7ede0f55bca4c0596f00fe89 diff --git a/jablib/src/main/resources/csl-locales b/jablib/src/main/resources/csl-locales index 7e137db2a55..e27762505af 160000 --- a/jablib/src/main/resources/csl-locales +++ b/jablib/src/main/resources/csl-locales @@ -1 +1 @@ -Subproject commit 7e137db2a55a724dbc7c406eb158f656f9a0f4ab +Subproject commit e27762505af6bfeedb68e0fb02c444b5f310b4e2 diff --git a/jablib/src/main/resources/csl-styles b/jablib/src/main/resources/csl-styles index 704ff9ffba5..458c4fabaad 160000 --- a/jablib/src/main/resources/csl-styles +++ b/jablib/src/main/resources/csl-styles @@ -1 +1 @@ -Subproject commit 704ff9ffba533dd67bb40607ef27514c2869fa09 +Subproject commit 458c4fabaad945959bb6025c5560bccb6b7026cf From 4dd6694c0a4e97c7435403a8181b67f14d85ab72 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Tue, 8 Jul 2025 14:55:22 +0530 Subject: [PATCH 04/24] Add @NonNull annotations and fix localization key warnings --- .../OpenSingleExternalFileAction.java | 28 +++++++++---------- .../main/resources/l10n/JabRef_en.properties | 3 ++ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java index 7721847a9ea..0c4eb9c3d09 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java @@ -1,7 +1,5 @@ package org.jabref.gui.maintable; -import java.util.Objects; - import org.jabref.gui.DialogService; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.fieldeditors.LinkedFileViewModel; @@ -11,6 +9,8 @@ import org.jabref.model.entry.BibEntry; import org.jabref.model.entry.LinkedFile; +import org.jspecify.annotations.NonNull; + public class OpenSingleExternalFileAction extends SimpleCommand { private final DialogService dialogService; @@ -20,18 +20,18 @@ public class OpenSingleExternalFileAction extends SimpleCommand { private final TaskExecutor taskExecutor; private final BibDatabaseContext databaseContext; - public OpenSingleExternalFileAction(DialogService dialogService, - GuiPreferences preferences, - BibEntry entry, - LinkedFile linkedFile, - TaskExecutor taskExecutor, - BibDatabaseContext databaseContext) { - this.dialogService = Objects.requireNonNull(dialogService); - this.preferences = Objects.requireNonNull(preferences); - this.entry = Objects.requireNonNull(entry); - this.linkedFile = Objects.requireNonNull(linkedFile); - this.taskExecutor = Objects.requireNonNull(taskExecutor); - this.databaseContext = Objects.requireNonNull(databaseContext); + public OpenSingleExternalFileAction(@NonNull DialogService dialogService, + @NonNull GuiPreferences preferences, + @NonNull BibEntry entry, + @NonNull LinkedFile linkedFile, + @NonNull TaskExecutor taskExecutor, + @NonNull BibDatabaseContext databaseContext) { + this.dialogService = dialogService; + this.preferences = preferences; + this.entry = entry; + this.linkedFile = linkedFile; + this.taskExecutor = taskExecutor; + this.databaseContext = databaseContext; this.setExecutable(true); } diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index 7a9e18dfa69..16253a8cb47 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -3001,3 +3001,6 @@ File\ '%0'\ already\ exists.\ Use\ -f\ or\ --force\ to\ overwrite.=File '%0' alr Pseudonymizing\ library\ '%0'...=Pseudonymizing library '%0'... Invalid\ output\ file\ type\ provided.=Invalid output file type provided. Saved\ %0.=Saved %0. + +Open\ all\ files=Open all files +Don't\ open=Don't open From dc0d67f7f715ebaece724d86ee0864641a69a350 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Tue, 8 Jul 2025 16:01:36 +0530 Subject: [PATCH 05/24] fix submodules --- jablib/src/main/abbrv.jabref.org | 2 +- jablib/src/main/resources/csl-locales | 2 +- jablib/src/main/resources/csl-styles | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jablib/src/main/abbrv.jabref.org b/jablib/src/main/abbrv.jabref.org index 333c2f19f96..193b23f48f1 160000 --- a/jablib/src/main/abbrv.jabref.org +++ b/jablib/src/main/abbrv.jabref.org @@ -1 +1 @@ -Subproject commit 333c2f19f96b760f7ede0f55bca4c0596f00fe89 +Subproject commit 193b23f48f1f137fe849781c2ecab6d32e27a86d diff --git a/jablib/src/main/resources/csl-locales b/jablib/src/main/resources/csl-locales index e27762505af..7e137db2a55 160000 --- a/jablib/src/main/resources/csl-locales +++ b/jablib/src/main/resources/csl-locales @@ -1 +1 @@ -Subproject commit e27762505af6bfeedb68e0fb02c444b5f310b4e2 +Subproject commit 7e137db2a55a724dbc7c406eb158f656f9a0f4ab diff --git a/jablib/src/main/resources/csl-styles b/jablib/src/main/resources/csl-styles index 458c4fabaad..704ff9ffba5 160000 --- a/jablib/src/main/resources/csl-styles +++ b/jablib/src/main/resources/csl-styles @@ -1 +1 @@ -Subproject commit 458c4fabaad945959bb6025c5560bccb6b7026cf +Subproject commit 704ff9ffba533dd67bb40607ef27514c2869fa09 From dd9ca5355f93b50a6dab70341bf646ae4b9c68ba Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Thu, 10 Jul 2025 17:17:23 +0530 Subject: [PATCH 06/24] Use database latest context, update button text, and add comments explaining code --- .../FulltextSearchResultsTab.java | 3 +-- .../maintable/OpenSelectedEntriesFilesAction.java | 6 ++++-- .../maintable/OpenSingleExternalFileAction.java | 14 +++++++++++--- .../src/main/resources/l10n/JabRef_en.properties | 2 +- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java b/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java index 3b35ada8da2..ded229a79bd 100644 --- a/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java +++ b/jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java @@ -180,9 +180,8 @@ private ContextMenu getFileContextMenu(LinkedFile file) { ContextMenu fileContextMenu = new ContextMenu(); fileContextMenu.getItems().add(actionFactory.createMenuItem( StandardActions.OPEN_FOLDER, new OpenFolderAction(dialogService, stateManager, preferences, entry, file, taskExecutor))); - BibDatabaseContext databaseContext = stateManager.getActiveDatabase().get(); fileContextMenu.getItems().add(actionFactory.createMenuItem( - StandardActions.OPEN_EXTERNAL_FILE, new OpenSingleExternalFileAction(dialogService, preferences, entry, file, taskExecutor, databaseContext))); + StandardActions.OPEN_EXTERNAL_FILE, new OpenSingleExternalFileAction(dialogService, preferences, entry, file, taskExecutor, stateManager))); return fileContextMenu; } diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java index 18cb719306b..2ddec22a770 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java @@ -41,6 +41,8 @@ public void execute() { stateManager.getActiveDatabase().ifPresent(databaseContext -> { final List selectedEntries = stateManager.getSelectedEntries(); + // Handle special case where a single entry with one linked file is selected. + // This is necessary because the right-click menu always uses OpenSelectedEntriesFilesAction. if (selectedEntries.size() == 1) { BibEntry entry = selectedEntries.getFirst(); List files = entry.getFiles(); @@ -52,7 +54,7 @@ public void execute() { entry, files.getFirst(), taskExecutor, - databaseContext + stateManager ).execute(); return; } @@ -79,7 +81,7 @@ public void execute() { Localization.lang("Opening large number of files"), Localization.lang("You are about to open %0 files. Continue?", linkedFileViewModelList.size()), Localization.lang("Open all files"), - Localization.lang("Don't open") + Localization.lang("Do not open") ); if (!continueOpening) { return; diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java index 0c4eb9c3d09..84fbf901ec9 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java @@ -1,6 +1,7 @@ package org.jabref.gui.maintable; import org.jabref.gui.DialogService; +import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; import org.jabref.gui.fieldeditors.LinkedFileViewModel; import org.jabref.gui.preferences.GuiPreferences; @@ -18,26 +19,33 @@ public class OpenSingleExternalFileAction extends SimpleCommand { private final BibEntry entry; private final LinkedFile linkedFile; private final TaskExecutor taskExecutor; - private final BibDatabaseContext databaseContext; + private final StateManager stateManager; public OpenSingleExternalFileAction(@NonNull DialogService dialogService, @NonNull GuiPreferences preferences, @NonNull BibEntry entry, @NonNull LinkedFile linkedFile, @NonNull TaskExecutor taskExecutor, - @NonNull BibDatabaseContext databaseContext) { + @NonNull StateManager stateManager) { this.dialogService = dialogService; this.preferences = preferences; this.entry = entry; this.linkedFile = linkedFile; this.taskExecutor = taskExecutor; - this.databaseContext = databaseContext; + this.stateManager = stateManager; this.setExecutable(true); } @Override public void execute() { + BibDatabaseContext databaseContext = stateManager.getActiveDatabase().orElse(null); + + if (databaseContext == null) { + dialogService.showErrorDialogAndWait("Cannot open file", "No active database found."); + return; + } + LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel( linkedFile, entry, diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index 16253a8cb47..c0fa8b75692 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -3003,4 +3003,4 @@ Invalid\ output\ file\ type\ provided.=Invalid output file type provided. Saved\ %0.=Saved %0. Open\ all\ files=Open all files -Don't\ open=Don't open +Do\ not\ open=Do not open From 6c654b6a025c17e7a1a7aed8072cc9bf19ad3832 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Jul 2025 22:25:44 +0000 Subject: [PATCH 07/24] Bump org.junit.platform:junit-platform-launcher in /versions (#13505) Bumps [org.junit.platform:junit-platform-launcher](https://github.com/junit-team/junit-framework) from 1.13.1 to 1.13.3. - [Release notes](https://github.com/junit-team/junit-framework/releases) - [Commits](https://github.com/junit-team/junit-framework/commits) --- updated-dependencies: - dependency-name: org.junit.platform:junit-platform-launcher dependency-version: 1.13.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Oliver Kopp From 77fd7a12cd3dacdcf45ed909c644dd589b8b953f Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Fri, 11 Jul 2025 13:28:11 +0530 Subject: [PATCH 08/24] Fix: clarify fallback logic, update dialog labels, and avoid null Optional fallback --- .../gui/maintable/OpenSelectedEntriesFilesAction.java | 9 +++++---- .../gui/maintable/OpenSingleExternalFileAction.java | 10 +++++----- jablib/src/main/resources/l10n/JabRef_en.properties | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java index 2ddec22a770..0e847aced91 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java @@ -41,8 +41,9 @@ public void execute() { stateManager.getActiveDatabase().ifPresent(databaseContext -> { final List selectedEntries = stateManager.getSelectedEntries(); - // Handle special case where a single entry with one linked file is selected. - // This is necessary because the right-click menu always uses OpenSelectedEntriesFilesAction. + // Special handling for the single-entry, single-file case. + // This fallback ensures the file opens immediately when triggered from the main table's right-click context menu, + // which always invokes OpenSelectedEntriesFilesAction regardless of the number of selected entries or files. if (selectedEntries.size() == 1) { BibEntry entry = selectedEntries.getFirst(); List files = entry.getFiles(); @@ -80,8 +81,8 @@ public void execute() { boolean continueOpening = dialogService.showConfirmationDialogAndWait( Localization.lang("Opening large number of files"), Localization.lang("You are about to open %0 files. Continue?", linkedFileViewModelList.size()), - Localization.lang("Open all files"), - Localization.lang("Do not open") + Localization.lang("Open all linked files"), + Localization.lang("Cancel file opening") ); if (!continueOpening) { return; diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java index 84fbf901ec9..695367eac7b 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java @@ -1,5 +1,7 @@ package org.jabref.gui.maintable; +import java.util.Optional; + import org.jabref.gui.DialogService; import org.jabref.gui.StateManager; import org.jabref.gui.actions.SimpleCommand; @@ -39,13 +41,11 @@ public OpenSingleExternalFileAction(@NonNull DialogService dialogService, @Override public void execute() { - BibDatabaseContext databaseContext = stateManager.getActiveDatabase().orElse(null); - - if (databaseContext == null) { - dialogService.showErrorDialogAndWait("Cannot open file", "No active database found."); + Optional databaseContextOptional = stateManager.getActiveDatabase(); + if (databaseContextOptional.isEmpty()) { return; } - + BibDatabaseContext databaseContext = databaseContextOptional.get(); LinkedFileViewModel linkedFileViewModel = new LinkedFileViewModel( linkedFile, entry, diff --git a/jablib/src/main/resources/l10n/JabRef_en.properties b/jablib/src/main/resources/l10n/JabRef_en.properties index c0fa8b75692..61a9888ede5 100644 --- a/jablib/src/main/resources/l10n/JabRef_en.properties +++ b/jablib/src/main/resources/l10n/JabRef_en.properties @@ -3002,5 +3002,5 @@ Pseudonymizing\ library\ '%0'...=Pseudonymizing library '%0'... Invalid\ output\ file\ type\ provided.=Invalid output file type provided. Saved\ %0.=Saved %0. -Open\ all\ files=Open all files -Do\ not\ open=Do not open +Open\ all\ linked\ files=Open all linked files +Cancel\ file\ opening=Cancel file opening From e717e073ec61108cdb3da8e96eb32f280cedc29c Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Fri, 18 Jul 2025 12:04:45 +0530 Subject: [PATCH 09/24] Fix: Make FileUtil.relativize symlink-aware for robust relative path resolution - Use real/canonical paths to ensure files are correctly recognized as within a directory, even through symlinks - Add and document unit tests covering symlinked and normal directory scenarios fixes #12995 --- .../org/jabref/logic/util/io/FileUtil.java | 28 +++++++++++++++++++ .../io/FileUtilSymlinkRelativizeTest.java | 4 +++ .../jabref/logic/util/io/FileUtilTest.java | 26 +++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index 9093acfe570..55c12a7186c 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -257,7 +257,26 @@ public static Path relativize(Path file, List directories) { return file; } + // Try "real path" matching to support symlinks + Optional realFileOpt = tryRealPath(file); + for (Path directory : directories) { + // Try real path comparison + Optional realDirOpt = tryRealPath(directory); + + if (realFileOpt.isPresent() && realDirOpt.isPresent()) { + Path realFile = realFileOpt.get(); + Path realDir = realDirOpt.get(); + if (realFile.startsWith(realDir)) { + // Figure out offset in original path structure, so the returned path is user-friendly and symlinks are preserved if possible + // Compute number of elements to drop from file, starting from directory length + int nameCountToDrop = realDir.getNameCount(); + Path relativePart = realFile.subpath(nameCountToDrop, realFile.getNameCount()); + return relativePart; + } + } + + // Fallback: original logic as before, for non-symlinked directories if (file.startsWith(directory)) { return directory.relativize(file); } @@ -265,6 +284,15 @@ public static Path relativize(Path file, List directories) { return file; } + private static Optional tryRealPath(Path path) { + try { + return Optional.of(path.toRealPath()); + } catch (IOException e) { + // maybe the file does not exist yet? fallback to absolute path. + return Optional.of(path.toAbsolutePath()); + } + } + /** * Converts an absolute file to a relative one, if possible. Returns the parameter file itself if no shortening is * possible. diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java new file mode 100644 index 00000000000..c1b08b022a8 --- /dev/null +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java @@ -0,0 +1,4 @@ +package org.jabref.logic.util.io; + +public class FileUtilSymlinkRelativizeTest { +} diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 6d63a6bd98f..71d1c7f1103 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -448,6 +448,32 @@ public void cTemp() { } } + @Test + void relativizeHandlesSymlinks(@TempDir Path tempDir) throws IOException { + assertTrue(!OS.WINDOWS); + + Path realDir = tempDir.resolve("real"); + Files.createDirectory(realDir); + + Path symlinkDir = tempDir.resolve("symlinked"); + Files.createSymbolicLink(symlinkDir, realDir); + + Path fileInRealDir = realDir.resolve("paper.pdf"); + Files.write(fileInRealDir, "dummy".getBytes(StandardCharsets.UTF_8)); + + Path rel = FileUtil.relativize(fileInRealDir, List.of(symlinkDir)); + assertEquals(Path.of("paper.pdf"), rel, "Should resolve to simple filename when accessed through symlink"); + Path fileViaSymlink = symlinkDir.resolve("paper.pdf"); + + Path rel2 = FileUtil.relativize(fileViaSymlink, List.of(symlinkDir)); + assertEquals(Path.of("paper.pdf"), rel2, "Should resolve to simple filename for file via symlink-path"); + Path outsideFile = tempDir.resolve("elsewhere.pdf"); + Files.write(outsideFile, "foo".getBytes(StandardCharsets.UTF_8)); + + Path rel3 = FileUtil.relativize(outsideFile, List.of(symlinkDir)); + assertEquals(outsideFile, rel3, "Unrelated files should not relativize"); + } + /** * @implNote Tests inspired by {@link org.jabref.model.database.BibDatabaseContextTest#getFileDirectoriesWithRelativeMetadata} */ From 22522dcbdac031109d39c754db6e3d67e0c1f29b Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sat, 19 Jul 2025 10:42:03 +0530 Subject: [PATCH 10/24] remove unnecessary comments and make test more clearer and maintainable --- .../OpenSelectedEntriesFilesAction.java | 52 ++++--------------- .../org/jabref/logic/util/io/FileUtil.java | 3 -- .../io/FileUtilSymlinkRelativizeTest.java | 4 -- .../jabref/logic/util/io/FileUtilTest.java | 2 +- 4 files changed, 12 insertions(+), 49 deletions(-) delete mode 100644 jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java diff --git a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java index 0e847aced91..9056f29dc43 100644 --- a/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java +++ b/jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java @@ -1,6 +1,5 @@ package org.jabref.gui.maintable; -import java.util.LinkedList; import java.util.List; import org.jabref.gui.DialogService; @@ -11,8 +10,6 @@ import org.jabref.gui.preferences.GuiPreferences; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.TaskExecutor; -import org.jabref.model.entry.BibEntry; -import org.jabref.model.entry.LinkedFile; public class OpenSelectedEntriesFilesAction extends SimpleCommand { @@ -39,44 +36,17 @@ public OpenSelectedEntriesFilesAction(DialogService dialogService, @Override public void execute() { stateManager.getActiveDatabase().ifPresent(databaseContext -> { - final List selectedEntries = stateManager.getSelectedEntries(); - - // Special handling for the single-entry, single-file case. - // This fallback ensures the file opens immediately when triggered from the main table's right-click context menu, - // which always invokes OpenSelectedEntriesFilesAction regardless of the number of selected entries or files. - if (selectedEntries.size() == 1) { - BibEntry entry = selectedEntries.getFirst(); - List files = entry.getFiles(); - - if (files.size() == 1) { - new OpenSingleExternalFileAction( - dialogService, - preferences, - entry, - files.getFirst(), - taskExecutor, - stateManager - ).execute(); - return; - } - } - - List linkedFileViewModelList = new LinkedList<>(); - - for (BibEntry entry : selectedEntries) { - for (LinkedFile linkedFile : entry.getFiles()) { - LinkedFileViewModel viewModel = new LinkedFileViewModel( - linkedFile, - entry, - databaseContext, - taskExecutor, - dialogService, - preferences); - - linkedFileViewModelList.add(viewModel); - } - } - + List linkedFileViewModelList = stateManager + .getSelectedEntries().stream() + .flatMap(entry -> entry.getFiles().stream() + .map(linkedFile -> new LinkedFileViewModel( + linkedFile, + entry, + databaseContext, + taskExecutor, + dialogService, + preferences))) + .toList(); if (linkedFileViewModelList.size() > FILES_LIMIT) { boolean continueOpening = dialogService.showConfirmationDialogAndWait( Localization.lang("Opening large number of files"), diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index 55c12a7186c..e95032d0f3b 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -256,12 +256,9 @@ public static Path relativize(Path file, List directories) { if (!file.isAbsolute()) { return file; } - - // Try "real path" matching to support symlinks Optional realFileOpt = tryRealPath(file); for (Path directory : directories) { - // Try real path comparison Optional realDirOpt = tryRealPath(directory); if (realFileOpt.isPresent() && realDirOpt.isPresent()) { diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java deleted file mode 100644 index c1b08b022a8..00000000000 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilSymlinkRelativizeTest.java +++ /dev/null @@ -1,4 +0,0 @@ -package org.jabref.logic.util.io; - -public class FileUtilSymlinkRelativizeTest { -} diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 71d1c7f1103..2346bb9805e 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -450,7 +450,7 @@ public void cTemp() { @Test void relativizeHandlesSymlinks(@TempDir Path tempDir) throws IOException { - assertTrue(!OS.WINDOWS); + assertFalse(OS.WINDOWS); Path realDir = tempDir.resolve("real"); Files.createDirectory(realDir); From 8c9257d1023a3f6f24a2d91681a9a5d107413c2d Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sat, 19 Jul 2025 10:57:10 +0530 Subject: [PATCH 11/24] Refactor to avoid using exceptions for normal control flow in tryRealPath and remove unnecessary comments --- .../java/org/jabref/logic/util/io/FileUtil.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index e95032d0f3b..419002f9d5c 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -265,15 +265,12 @@ public static Path relativize(Path file, List directories) { Path realFile = realFileOpt.get(); Path realDir = realDirOpt.get(); if (realFile.startsWith(realDir)) { - // Figure out offset in original path structure, so the returned path is user-friendly and symlinks are preserved if possible - // Compute number of elements to drop from file, starting from directory length int nameCountToDrop = realDir.getNameCount(); Path relativePart = realFile.subpath(nameCountToDrop, realFile.getNameCount()); return relativePart; } } - // Fallback: original logic as before, for non-symlinked directories if (file.startsWith(directory)) { return directory.relativize(file); } @@ -282,10 +279,13 @@ public static Path relativize(Path file, List directories) { } private static Optional tryRealPath(Path path) { - try { - return Optional.of(path.toRealPath()); - } catch (IOException e) { - // maybe the file does not exist yet? fallback to absolute path. + if (Files.exists(path)) { + try { + return Optional.of(path.toRealPath()); + } catch (IOException e) { + return Optional.empty(); + } + } else { return Optional.of(path.toAbsolutePath()); } } From 1d236aa69019b9c3af85ddfdcc82749336240eb1 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sun, 20 Jul 2025 15:37:36 +0530 Subject: [PATCH 12/24] Refactored symlink tests for independence, used Files.writeString for simplicity, and ensured all edge cases from issue #12995 are now covered. --- .../jabref/logic/util/io/FileUtilTest.java | 55 ++++++++++++------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 2346bb9805e..988045ee69a 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -4,7 +4,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardOpenOption; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -54,11 +53,11 @@ void setUpViewModel(@TempDir Path temporaryFolder) throws IOException { existingTestFile = subDir.resolve("existingTestFile.txt"); Files.createFile(existingTestFile); - Files.write(existingTestFile, "existingTestFile.txt".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND); + Files.writeString(existingTestFile, "existingTestFile.txt"); otherExistingTestFile = subDir.resolve("otherExistingTestFile.txt"); Files.createFile(otherExistingTestFile); - Files.write(otherExistingTestFile, "otherExistingTestFile.txt".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND); + Files.writeString(otherExistingTestFile, "otherExistingTestFile.txt"); } @Test @@ -448,30 +447,44 @@ public void cTemp() { } } - @Test - void relativizeHandlesSymlinks(@TempDir Path tempDir) throws IOException { - assertFalse(OS.WINDOWS); - - Path realDir = tempDir.resolve("real"); - Files.createDirectory(realDir); + @ParameterizedTest(name = "{3}") + @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") + @MethodSource("symlinkRelativizeScenarios") + void relativizeSymlinkAdvanced(Path file, List directories, Path expected, String message) { + Path result = FileUtil.relativize(file, directories); + assertEquals(expected, result, message); + } - Path symlinkDir = tempDir.resolve("symlinked"); + static Stream symlinkRelativizeScenarios() throws IOException { + Path tempDir = Files.createTempDirectory("symlinktest-"); + Path realDir = tempDir.resolve("realDir"); + Files.createDirectories(realDir); + Path symlinkDir = tempDir.resolve("symlinkDir"); Files.createSymbolicLink(symlinkDir, realDir); + Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); - Path fileInRealDir = realDir.resolve("paper.pdf"); - Files.write(fileInRealDir, "dummy".getBytes(StandardCharsets.UTF_8)); + Path chainReal = tempDir.resolve("chainReal"); + Files.createDirectories(chainReal); + Path chainLink2 = tempDir.resolve("chainLink2"); + Path chainLink1 = tempDir.resolve("chainLink1"); + Files.createSymbolicLink(chainLink2, chainReal); + Files.createSymbolicLink(chainLink1, chainLink2); + Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); - Path rel = FileUtil.relativize(fileInRealDir, List.of(symlinkDir)); - assertEquals(Path.of("paper.pdf"), rel, "Should resolve to simple filename when accessed through symlink"); - Path fileViaSymlink = symlinkDir.resolve("paper.pdf"); + Path nestedDir = realDir.resolve("nested"); + Files.createDirectories(nestedDir); + Path nestedSymlink = realDir.resolve("nestedLink"); + Files.createSymbolicLink(nestedSymlink, nestedDir); + Path nestedFile = Files.createFile((nestedDir.resolve("nested.pdf"))); - Path rel2 = FileUtil.relativize(fileViaSymlink, List.of(symlinkDir)); - assertEquals(Path.of("paper.pdf"), rel2, "Should resolve to simple filename for file via symlink-path"); - Path outsideFile = tempDir.resolve("elsewhere.pdf"); - Files.write(outsideFile, "foo".getBytes(StandardCharsets.UTF_8)); + Path outsideFile = Files.createFile(tempDir.resolve("outside.pdf")); - Path rel3 = FileUtil.relativize(outsideFile, List.of(symlinkDir)); - assertEquals(outsideFile, rel3, "Unrelated files should not relativize"); + return Stream.of( + Arguments.of(simpleFile, List.of(symlinkDir), Path.of("simple.pdf"), "Simple symlink resolves to relative"), + Arguments.of(chainedFile, List.of(chainLink1), Path.of("chained.pdf"), "Chained symlink resolves to relative"), + Arguments.of(nestedFile, List.of(nestedSymlink), Path.of("nested.pdf"), "Nested symlink resolves to relative"), + Arguments.of(outsideFile, List.of(symlinkDir), outsideFile, "Unrelated file remains absolute") + ); } /** From 5124b5ffbd1ca6b326d82320989f34d491109e31 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sun, 20 Jul 2025 16:17:31 +0530 Subject: [PATCH 13/24] Fix code style issues with OpenRewrite and update symlink tests --- jablib/src/main/resources/csl-locales | 2 +- jablib/src/main/resources/csl-styles | 2 +- .../org/jabref/logic/util/io/FileUtilTest.java | 17 ++++++++--------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/jablib/src/main/resources/csl-locales b/jablib/src/main/resources/csl-locales index 3bad4339367..7e137db2a55 160000 --- a/jablib/src/main/resources/csl-locales +++ b/jablib/src/main/resources/csl-locales @@ -1 +1 @@ -Subproject commit 3bad433936712a0c41ffe300442e7f519b51c89f +Subproject commit 7e137db2a55a724dbc7c406eb158f656f9a0f4ab diff --git a/jablib/src/main/resources/csl-styles b/jablib/src/main/resources/csl-styles index 59f124dc674..704ff9ffba5 160000 --- a/jablib/src/main/resources/csl-styles +++ b/jablib/src/main/resources/csl-styles @@ -1 +1 @@ -Subproject commit 59f124dc67441738d8823bc59d517edef8f43e06 +Subproject commit 704ff9ffba533dd67bb40607ef27514c2869fa09 diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 988045ee69a..27f4b21286e 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -447,7 +447,7 @@ public void cTemp() { } } - @ParameterizedTest(name = "{3}") + @ParameterizedTest @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") @MethodSource("symlinkRelativizeScenarios") void relativizeSymlinkAdvanced(Path file, List directories, Path expected, String message) { @@ -456,17 +456,16 @@ void relativizeSymlinkAdvanced(Path file, List directories, Path expected, } static Stream symlinkRelativizeScenarios() throws IOException { - Path tempDir = Files.createTempDirectory("symlinktest-"); - Path realDir = tempDir.resolve("realDir"); + Path realDir = bibTempDir.resolve("realDir"); Files.createDirectories(realDir); - Path symlinkDir = tempDir.resolve("symlinkDir"); + Path symlinkDir = bibTempDir.resolve("symlinkDir"); Files.createSymbolicLink(symlinkDir, realDir); Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); - Path chainReal = tempDir.resolve("chainReal"); + Path chainReal = bibTempDir.resolve("chainReal"); Files.createDirectories(chainReal); - Path chainLink2 = tempDir.resolve("chainLink2"); - Path chainLink1 = tempDir.resolve("chainLink1"); + Path chainLink2 = bibTempDir.resolve("chainLink2"); + Path chainLink1 = bibTempDir.resolve("chainLink1"); Files.createSymbolicLink(chainLink2, chainReal); Files.createSymbolicLink(chainLink1, chainLink2); Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); @@ -475,9 +474,9 @@ static Stream symlinkRelativizeScenarios() throws IOException { Files.createDirectories(nestedDir); Path nestedSymlink = realDir.resolve("nestedLink"); Files.createSymbolicLink(nestedSymlink, nestedDir); - Path nestedFile = Files.createFile((nestedDir.resolve("nested.pdf"))); + Path nestedFile = Files.createFile(nestedDir.resolve("nested.pdf")); - Path outsideFile = Files.createFile(tempDir.resolve("outside.pdf")); + Path outsideFile = Files.createFile(bibTempDir.resolve("outside.pdf")); return Stream.of( Arguments.of(simpleFile, List.of(symlinkDir), Path.of("simple.pdf"), "Simple symlink resolves to relative"), From ad92c0d598392b1f55776087d4ecbdaa9d401742 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sun, 20 Jul 2025 16:21:01 +0530 Subject: [PATCH 14/24] Fix submodules --- jablib/src/main/abbrv.jabref.org | 2 +- jablib/src/main/resources/csl-locales | 2 +- jablib/src/main/resources/csl-styles | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jablib/src/main/abbrv.jabref.org b/jablib/src/main/abbrv.jabref.org index 193b23f48f1..333c2f19f96 160000 --- a/jablib/src/main/abbrv.jabref.org +++ b/jablib/src/main/abbrv.jabref.org @@ -1 +1 @@ -Subproject commit 193b23f48f1f137fe849781c2ecab6d32e27a86d +Subproject commit 333c2f19f96b760f7ede0f55bca4c0596f00fe89 diff --git a/jablib/src/main/resources/csl-locales b/jablib/src/main/resources/csl-locales index 7e137db2a55..e27762505af 160000 --- a/jablib/src/main/resources/csl-locales +++ b/jablib/src/main/resources/csl-locales @@ -1 +1 @@ -Subproject commit 7e137db2a55a724dbc7c406eb158f656f9a0f4ab +Subproject commit e27762505af6bfeedb68e0fb02c444b5f310b4e2 diff --git a/jablib/src/main/resources/csl-styles b/jablib/src/main/resources/csl-styles index 704ff9ffba5..458c4fabaad 160000 --- a/jablib/src/main/resources/csl-styles +++ b/jablib/src/main/resources/csl-styles @@ -1 +1 @@ -Subproject commit 704ff9ffba533dd67bb40607ef27514c2869fa09 +Subproject commit 458c4fabaad945959bb6025c5560bccb6b7026cf From cb314a21f277b28cfc575cc2126cdb1d4d38fb2b Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sun, 20 Jul 2025 16:27:46 +0530 Subject: [PATCH 15/24] Fix submodules --- jablib/src/main/abbrv.jabref.org | 2 +- jablib/src/main/resources/csl-locales | 2 +- jablib/src/main/resources/csl-styles | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/jablib/src/main/abbrv.jabref.org b/jablib/src/main/abbrv.jabref.org index 333c2f19f96..193b23f48f1 160000 --- a/jablib/src/main/abbrv.jabref.org +++ b/jablib/src/main/abbrv.jabref.org @@ -1 +1 @@ -Subproject commit 333c2f19f96b760f7ede0f55bca4c0596f00fe89 +Subproject commit 193b23f48f1f137fe849781c2ecab6d32e27a86d diff --git a/jablib/src/main/resources/csl-locales b/jablib/src/main/resources/csl-locales index e27762505af..3bad4339367 160000 --- a/jablib/src/main/resources/csl-locales +++ b/jablib/src/main/resources/csl-locales @@ -1 +1 @@ -Subproject commit e27762505af6bfeedb68e0fb02c444b5f310b4e2 +Subproject commit 3bad433936712a0c41ffe300442e7f519b51c89f diff --git a/jablib/src/main/resources/csl-styles b/jablib/src/main/resources/csl-styles index 458c4fabaad..59f124dc674 160000 --- a/jablib/src/main/resources/csl-styles +++ b/jablib/src/main/resources/csl-styles @@ -1 +1 @@ -Subproject commit 458c4fabaad945959bb6025c5560bccb6b7026cf +Subproject commit 59f124dc67441738d8823bc59d517edef8f43e06 From b7f7587af284d7ccf7ece2a3ce646760e55a74bb Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 4 Aug 2025 23:17:06 +0200 Subject: [PATCH 16/24] Fix typo --- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index 419002f9d5c..aeee112dfa4 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -220,7 +220,7 @@ public static List uniquePathSubstrings(List paths) { * @param pathToSourceFile Path Source file * @param pathToDestinationFile Path Destination file * @param replaceExisting boolean Determines whether the copy goes on even if the file exists. - * @return boolean Whether the copy succeeded, or was stopped due to the file already existing. + * @return boolean Whether the copy succeeded or was stopped due to the file already existing. */ public static boolean copyFile(Path pathToSourceFile, Path pathToDestinationFile, boolean replaceExisting) { // Check if the file already exists. From 4270261fdce13afe429d59314411ef2fbb4172f3 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Mon, 4 Aug 2025 23:32:48 +0200 Subject: [PATCH 17/24] Refactor tests --- .../jabref/logic/util/io/FileUtilTest.java | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 27f4b21286e..3506dbcc56a 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -4,6 +4,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -37,8 +38,9 @@ @AllowedToUseLogic("uses OS from logic package") class FileUtilTest { + // Needs to be class variable, otherwise "@MethodSource" does not work @TempDir - static Path bibTempDir; + private static Path bibTempDir; private final Path nonExistingTestPath = Path.of("nonExistingTestPath"); private Path existingTestFile; @@ -449,41 +451,52 @@ public void cTemp() { @ParameterizedTest @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") - @MethodSource("symlinkRelativizeScenarios") - void relativizeSymlinkAdvanced(Path file, List directories, Path expected, String message) { + @MethodSource + void relativizeSymlinks(Path file, List directories, Path expected, String message) { Path result = FileUtil.relativize(file, directories); assertEquals(expected, result, message); } - static Stream symlinkRelativizeScenarios() throws IOException { + /// Tests for issue + static Stream relativizeSymlinks() throws IOException { + List result = new ArrayList<>(); + Path realDir = bibTempDir.resolve("realDir"); Files.createDirectories(realDir); + + // symlinkDir -> realDir + // realDir/simple.pdf + Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); Path symlinkDir = bibTempDir.resolve("symlinkDir"); Files.createSymbolicLink(symlinkDir, realDir); - Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); + result.add(Arguments.of(simpleFile, List.of(symlinkDir), Path.of("simple.pdf"), "Simple symlink resolves to relative")); + // chainLink1 -> chainLink2 -> chainReal + // chainReal/chained.pdf Path chainReal = bibTempDir.resolve("chainReal"); Files.createDirectories(chainReal); + Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); Path chainLink2 = bibTempDir.resolve("chainLink2"); - Path chainLink1 = bibTempDir.resolve("chainLink1"); Files.createSymbolicLink(chainLink2, chainReal); + Path chainLink1 = bibTempDir.resolve("chainLink1"); Files.createSymbolicLink(chainLink1, chainLink2); - Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); + result.add(Arguments.of(chainedFile, List.of(chainLink1), Path.of("chained.pdf"), "Chained symlink resolves to relative")); + // realDir/nestedLink -> realDir/nested + // realDir/nested/nested.pdf Path nestedDir = realDir.resolve("nested"); Files.createDirectories(nestedDir); + Path nestedFile = Files.createFile(nestedDir.resolve("nested.pdf")); Path nestedSymlink = realDir.resolve("nestedLink"); Files.createSymbolicLink(nestedSymlink, nestedDir); - Path nestedFile = Files.createFile(nestedDir.resolve("nested.pdf")); + result.add(Arguments.of(nestedFile, List.of(nestedSymlink), Path.of("nested.pdf"), "Nested symlink resolves to relative")); + // symlinkDir -> realDir + // outside.pdf Path outsideFile = Files.createFile(bibTempDir.resolve("outside.pdf")); + result.add(Arguments.of(outsideFile, List.of(symlinkDir), outsideFile, "Unrelated file remains absolute")); - return Stream.of( - Arguments.of(simpleFile, List.of(symlinkDir), Path.of("simple.pdf"), "Simple symlink resolves to relative"), - Arguments.of(chainedFile, List.of(chainLink1), Path.of("chained.pdf"), "Chained symlink resolves to relative"), - Arguments.of(nestedFile, List.of(nestedSymlink), Path.of("nested.pdf"), "Nested symlink resolves to relative"), - Arguments.of(outsideFile, List.of(symlinkDir), outsideFile, "Unrelated file remains absolute") - ); + return result.stream(); } /** From 18fa79790c1af662b092d3650f4400c3bd7d66fa Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sun, 17 Aug 2025 15:38:49 +0530 Subject: [PATCH 18/24] Add comments to explain the logic to not use directory.relativize() --- jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index 419002f9d5c..0fcf32f8f2f 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -264,6 +264,8 @@ public static Path relativize(Path file, List directories) { if (realFileOpt.isPresent() && realDirOpt.isPresent()) { Path realFile = realFileOpt.get(); Path realDir = realDirOpt.get(); + // Avoid directory.relativize() because it breaks on some symlink edge cases. + // Instead, resolve to canonical paths and drop the directory’s name elements manually to get a correct relative path. if (realFile.startsWith(realDir)) { int nameCountToDrop = realDir.getNameCount(); Path relativePart = realFile.subpath(nameCountToDrop, realFile.getNameCount()); From 618e12ab3deee88ef56089e30ad617e0eaba8883 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Sun, 17 Aug 2025 15:59:58 +0530 Subject: [PATCH 19/24] Removed unneccesary comment --- jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 3506dbcc56a..2163dee2f86 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -38,7 +38,6 @@ @AllowedToUseLogic("uses OS from logic package") class FileUtilTest { - // Needs to be class variable, otherwise "@MethodSource" does not work @TempDir private static Path bibTempDir; From 19d7cb3773f445a1e0926e0d955cb4f927d6fcf0 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Mon, 18 Aug 2025 18:43:39 +0530 Subject: [PATCH 20/24] Add ignored test for symlink escape case (#12995 comment) --- .../org/jabref/logic/util/io/FileUtil.java | 14 ++++---- .../jabref/logic/util/io/FileUtilTest.java | 34 ++++++++++++++----- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index d7dd4f29cc4..e0581c78b7b 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -153,7 +153,7 @@ public static Optional getUniquePathDirectory(List paths, Path c List uniquePathParts = uniquePathSubstrings(paths); return uniquePathParts.stream() .filter(part -> comparePath.toString().contains(part) - && !part.equals(fileName) && part.contains(File.separator)) + && !part.equals(fileName) && part.contains(File.separator)) .findFirst() .map(part -> part.substring(0, part.lastIndexOf(File.separator))); } @@ -397,9 +397,9 @@ public static String createDirNameFromPattern(BibDatabase database, BibEntry ent public static Optional findSingleFileRecursively(String filename, Path rootDirectory) { try (Stream pathStream = Files.walk(rootDirectory)) { return pathStream - .filter(Files::isRegularFile) - .filter(f -> f.getFileName().toString().equals(filename)) - .findFirst(); + .filter(Files::isRegularFile) + .filter(f -> f.getFileName().toString().equals(filename)) + .findFirst(); } catch (UncheckedIOException | IOException ex) { LOGGER.error("Error trying to locate the file {} inside the directory {}", filename, rootDirectory, ex); } @@ -606,9 +606,9 @@ public static String shortenFileName(String fileName, Integer maxLength) { numCharsAfterEllipsis = Math.min(numCharsAfterEllipsis, name.length() - numCharsBeforeEllipsis); return name.substring(0, numCharsBeforeEllipsis) + - ELLIPSIS + - name.substring(name.length() - numCharsAfterEllipsis) + - extension; + ELLIPSIS + + name.substring(name.length() - numCharsAfterEllipsis) + + extension; } public static boolean isCharLegal(char c) { diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 2163dee2f86..009f784a240 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -222,16 +222,16 @@ void getFileNameWithMultipleDotsString() { @Test @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Assumed path separator is /") void uniquePathSubstrings() { - List paths = List.of("C:/uniquefile.bib", - "C:/downloads/filename.bib", - "C:/mypaper/bib/filename.bib", - "C:/external/mypaper/bib/filename.bib", - ""); + List paths = List.of("C:/uniquefile.bib", + "C:/downloads/filename.bib", + "C:/mypaper/bib/filename.bib", + "C:/external/mypaper/bib/filename.bib", + ""); List uniqPath = List.of("uniquefile.bib", - "downloads/filename.bib", - "C:/mypaper/bib/filename.bib", - "external/mypaper/bib/filename.bib", - ""); + "downloads/filename.bib", + "C:/mypaper/bib/filename.bib", + "external/mypaper/bib/filename.bib", + ""); List result = FileUtil.uniquePathSubstrings(paths); assertEquals(uniqPath, result); @@ -452,6 +452,9 @@ public void cTemp() { @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") @MethodSource void relativizeSymlinks(Path file, List directories, Path expected, String message) { + if (message.startsWith("IGNORED")) { + org.junit.jupiter.api.Assumptions.assumeTrue(false, message); + } Path result = FileUtil.relativize(file, directories); assertEquals(expected, result, message); } @@ -495,6 +498,19 @@ static Stream relativizeSymlinks() throws IOException { Path outsideFile = Files.createFile(bibTempDir.resolve("outside.pdf")); result.add(Arguments.of(outsideFile, List.of(symlinkDir), outsideFile, "Unrelated file remains absolute")); + // symlink chain escaping base dir (ignored test case, see #12995 issue comment) + Path veryPrivate = bibTempDir.resolve("veryprivate"); + Files.createDirectories(veryPrivate); + Path secretFile = Files.createFile(veryPrivate.resolve("a.pdf")); + Path expensive = bibTempDir.resolve("expensive"); + Files.createSymbolicLink(expensive, veryPrivate); + Path things = bibTempDir.resolve("things"); + Files.createSymbolicLink(things, expensive); + Path libDir = bibTempDir.resolve("lib"); + Files.createDirectories(libDir); + Path bibFile = Files.createFile(libDir.resolve("bib.bib")); + result.add(Arguments.of(secretFile, List.of(things), secretFile, "IGNORED: Symlink chain escaping base dir (#12995 comment)")); + return result.stream(); } From be8b7f66cadd572e98c2a8f7bff63f68666bf044 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Fri, 29 Aug 2025 00:01:08 +0530 Subject: [PATCH 21/24] Tests: split relativizeSymlinks() parameterized test into separate test cases for better readability --- .../jabref/logic/util/io/FileUtilTest.java | 77 +++++++++++-------- 1 file changed, 46 insertions(+), 31 deletions(-) diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index 009f784a240..7e9d1f474ec 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -4,7 +4,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -448,70 +447,86 @@ public void cTemp() { } } - @ParameterizedTest - @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") - @MethodSource - void relativizeSymlinks(Path file, List directories, Path expected, String message) { - if (message.startsWith("IGNORED")) { - org.junit.jupiter.api.Assumptions.assumeTrue(false, message); - } - Path result = FileUtil.relativize(file, directories); - assertEquals(expected, result, message); - } - /// Tests for issue - static Stream relativizeSymlinks() throws IOException { - List result = new ArrayList<>(); - + @Test + @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") + void simpleRelativizeSymlinks() throws IOException { Path realDir = bibTempDir.resolve("realDir"); Files.createDirectories(realDir); - // symlinkDir -> realDir - // realDir/simple.pdf Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); Path symlinkDir = bibTempDir.resolve("symlinkDir"); Files.createSymbolicLink(symlinkDir, realDir); - result.add(Arguments.of(simpleFile, List.of(symlinkDir), Path.of("simple.pdf"), "Simple symlink resolves to relative")); - // chainLink1 -> chainLink2 -> chainReal - // chainReal/chained.pdf + Path result = FileUtil.relativize(simpleFile, List.of(symlinkDir)); + assertEquals(Path.of("simple.pdf"), result, "Simple symlink resolves to relative"); + } + + @Test + @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") + void chainedRelativizeSymlinks() throws IOException { Path chainReal = bibTempDir.resolve("chainReal"); Files.createDirectories(chainReal); + Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); Path chainLink2 = bibTempDir.resolve("chainLink2"); Files.createSymbolicLink(chainLink2, chainReal); Path chainLink1 = bibTempDir.resolve("chainLink1"); Files.createSymbolicLink(chainLink1, chainLink2); - result.add(Arguments.of(chainedFile, List.of(chainLink1), Path.of("chained.pdf"), "Chained symlink resolves to relative")); - // realDir/nestedLink -> realDir/nested - // realDir/nested/nested.pdf + Path result = FileUtil.relativize(chainedFile, List.of(chainLink1)); + assertEquals(Path.of("chained.pdf"), result, "Chained symlink resolves to relative"); + } + + @Test + @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") + void nestedRelativizeSymlinks() throws IOException { + Path realDir = bibTempDir.resolve("realDir"); + Files.createDirectories(realDir); + Path nestedDir = realDir.resolve("nested"); Files.createDirectories(nestedDir); Path nestedFile = Files.createFile(nestedDir.resolve("nested.pdf")); Path nestedSymlink = realDir.resolve("nestedLink"); Files.createSymbolicLink(nestedSymlink, nestedDir); - result.add(Arguments.of(nestedFile, List.of(nestedSymlink), Path.of("nested.pdf"), "Nested symlink resolves to relative")); - // symlinkDir -> realDir - // outside.pdf + Path result = FileUtil.relativize(nestedFile, List.of(nestedSymlink)); + assertEquals(Path.of("nested.pdf"), result, "Nested symlink resolves to relative"); + } + + @Test + @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") + void unrelatedFileRemainsAbsolute() throws IOException { + Path realDir = bibTempDir.resolve("realDir"); + Files.createDirectories(realDir); + Path symlinkDir = bibTempDir.resolve("symlinkDir"); + Files.createSymbolicLink(symlinkDir, realDir); + Path outsideFile = Files.createFile(bibTempDir.resolve("outside.pdf")); - result.add(Arguments.of(outsideFile, List.of(symlinkDir), outsideFile, "Unrelated file remains absolute")); - // symlink chain escaping base dir (ignored test case, see #12995 issue comment) + Path result = FileUtil.relativize(outsideFile, List.of(symlinkDir)); + assertEquals(outsideFile, result, "Unrelated file remains absolute"); + } + + @Test + @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") + void symlinkEscapeCaseIgnored() throws IOException { Path veryPrivate = bibTempDir.resolve("veryprivate"); Files.createDirectories(veryPrivate); Path secretFile = Files.createFile(veryPrivate.resolve("a.pdf")); + Path expensive = bibTempDir.resolve("expensive"); Files.createSymbolicLink(expensive, veryPrivate); Path things = bibTempDir.resolve("things"); Files.createSymbolicLink(things, expensive); + Path libDir = bibTempDir.resolve("lib"); Files.createDirectories(libDir); - Path bibFile = Files.createFile(libDir.resolve("bib.bib")); - result.add(Arguments.of(secretFile, List.of(things), secretFile, "IGNORED: Symlink chain escaping base dir (#12995 comment)")); + Files.createFile(libDir.resolve("bib.bib")); - return result.stream(); + org.junit.jupiter.api.Assumptions.assumeTrue(false, "IGNORED: Symlink chain escaping base dir, see "); + Path result = FileUtil.relativize(secretFile, List.of(things)); + assertEquals(secretFile, result); } /** From c0c635d7bdae132682d110df857d655efec65a54 Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Fri, 29 Aug 2025 00:30:04 +0530 Subject: [PATCH 22/24] resolved checkstyle issues --- .../java/org/jabref/logic/util/io/FileUtil.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java index bf558434361..f87053f9ef7 100644 --- a/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java +++ b/jablib/src/main/java/org/jabref/logic/util/io/FileUtil.java @@ -253,20 +253,6 @@ public static Path relativize(Path file, List directories) { Optional realFileOpt = toRealPath(file); for (Path directory : directories) { - Optional realDirOpt = tryRealPath(directory); - - if (realFileOpt.isPresent() && realDirOpt.isPresent()) { - Path realFile = realFileOpt.get(); - Path realDir = realDirOpt.get(); - // Avoid directory.relativize() because it breaks on some symlink edge cases. - // Instead, resolve to canonical paths and drop the directory’s name elements manually to get a correct relative path. - if (realFile.startsWith(realDir)) { - int nameCountToDrop = realDir.getNameCount(); - Path relativePart = realFile.subpath(nameCountToDrop, realFile.getNameCount()); - return relativePart; - } - } - if (file.startsWith(directory)) { return directory.relativize(file); } From 44c47f11421fe981ef428f8c36af2baa261f05ef Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Fri, 29 Aug 2025 00:55:06 +0530 Subject: [PATCH 23/24] Resolved failed test cases issue and removed unused imports --- .../java/org/jabref/logic/util/io/FileUtilTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index d4aedb4dbbb..dd49813d435 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -4,9 +4,9 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -452,7 +452,7 @@ public void cTemp() { @Test @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") void simpleRelativizeSymlinks() throws IOException { - Path realDir = bibTempDir.resolve("realDir"); + Path realDir = bibTempDir.resolve("realDir_" + UUID.randomUUID()); Files.createDirectories(realDir); Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); @@ -466,7 +466,7 @@ void simpleRelativizeSymlinks() throws IOException { @Test @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") void chainedRelativizeSymlinks() throws IOException { - Path chainReal = bibTempDir.resolve("chainReal"); + Path chainReal = bibTempDir.resolve("chainReal_" + UUID.randomUUID()); Files.createDirectories(chainReal); Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); @@ -482,7 +482,7 @@ void chainedRelativizeSymlinks() throws IOException { @Test @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") void nestedRelativizeSymlinks() throws IOException { - Path realDir = bibTempDir.resolve("realDir"); + Path realDir = bibTempDir.resolve("realDir_" + UUID.randomUUID()); Files.createDirectories(realDir); Path nestedDir = realDir.resolve("nested"); @@ -498,7 +498,7 @@ void nestedRelativizeSymlinks() throws IOException { @Test @DisabledOnOs(value = org.junit.jupiter.api.condition.OS.WINDOWS, disabledReason = "Symlink behavior unreliable on windows") void unrelatedFileRemainsAbsolute() throws IOException { - Path realDir = bibTempDir.resolve("realDir"); + Path realDir = bibTempDir.resolve("realDir_" + UUID.randomUUID()); Files.createDirectories(realDir); Path symlinkDir = bibTempDir.resolve("symlinkDir"); Files.createSymbolicLink(symlinkDir, realDir); From 3b763cb44643829883a1e4acfc47cf0a8138903b Mon Sep 17 00:00:00 2001 From: Muskan Raghav Date: Fri, 29 Aug 2025 01:09:19 +0530 Subject: [PATCH 24/24] fix failing tests in jablib module --- .../java/org/jabref/logic/util/io/FileUtilTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java index dd49813d435..8bff7f1e4cd 100644 --- a/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java +++ b/jablib/src/test/java/org/jabref/logic/util/io/FileUtilTest.java @@ -456,7 +456,7 @@ void simpleRelativizeSymlinks() throws IOException { Files.createDirectories(realDir); Path simpleFile = Files.createFile(realDir.resolve("simple.pdf")); - Path symlinkDir = bibTempDir.resolve("symlinkDir"); + Path symlinkDir = bibTempDir.resolve("symlinkDir_" + UUID.randomUUID()); Files.createSymbolicLink(symlinkDir, realDir); Path result = FileUtil.relativize(simpleFile, List.of(symlinkDir)); @@ -470,9 +470,9 @@ void chainedRelativizeSymlinks() throws IOException { Files.createDirectories(chainReal); Path chainedFile = Files.createFile(chainReal.resolve("chained.pdf")); - Path chainLink2 = bibTempDir.resolve("chainLink2"); + Path chainLink2 = bibTempDir.resolve("chainLink2_" + UUID.randomUUID()); Files.createSymbolicLink(chainLink2, chainReal); - Path chainLink1 = bibTempDir.resolve("chainLink1"); + Path chainLink1 = bibTempDir.resolve("chainLink1_" + UUID.randomUUID()); Files.createSymbolicLink(chainLink1, chainLink2); Path result = FileUtil.relativize(chainedFile, List.of(chainLink1)); @@ -485,10 +485,10 @@ void nestedRelativizeSymlinks() throws IOException { Path realDir = bibTempDir.resolve("realDir_" + UUID.randomUUID()); Files.createDirectories(realDir); - Path nestedDir = realDir.resolve("nested"); + Path nestedDir = realDir.resolve("nested_" + UUID.randomUUID()); Files.createDirectories(nestedDir); Path nestedFile = Files.createFile(nestedDir.resolve("nested.pdf")); - Path nestedSymlink = realDir.resolve("nestedLink"); + Path nestedSymlink = realDir.resolve("nestedLink_" + UUID.randomUUID()); Files.createSymbolicLink(nestedSymlink, nestedDir); Path result = FileUtil.relativize(nestedFile, List.of(nestedSymlink)); @@ -500,7 +500,7 @@ void nestedRelativizeSymlinks() throws IOException { void unrelatedFileRemainsAbsolute() throws IOException { Path realDir = bibTempDir.resolve("realDir_" + UUID.randomUUID()); Files.createDirectories(realDir); - Path symlinkDir = bibTempDir.resolve("symlinkDir"); + Path symlinkDir = bibTempDir.resolve("symlinkDir_" + UUID.randomUUID()); Files.createSymbolicLink(symlinkDir, realDir); Path outsideFile = Files.createFile(bibTempDir.resolve("outside.pdf"));