Skip to content

Commit

Permalink
Caught an error when accessing an invalid path. fixes JabRef#10548 (J…
Browse files Browse the repository at this point in the history
…abRef#12038)

* Caught an error when accessing an invalid path. fixes JabRef#10548

* Fixed typo in issue num

* Moved changelog entry to unreleased section

* Moved try-catch and added tests

* Fix checkstyle

* Fix checkstyle again

* Fix checkstyle fr this time

* Discard changes to external-libraries.md

* Result of "rewriteRun"

* Add link to issue

* Use VisibleForTesting annotation

* Remove Jimfs depdencency

---------

Co-authored-by: Tom Martin <[email protected]>
Co-authored-by: Oliver Kopp <[email protected]>
  • Loading branch information
3 people authored Oct 23, 2024
1 parent b90ef75 commit 60e9ded
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We fixed an issue where <kbd>Tab</kbd> cannot be used to jump to next field in some single-line fields. [#11785](https://github.com/JabRef/jabref/issues/11785)
- We fixed an issue where it was not possible to select selecting content of other user's comments.[#11106](https://github.com/JabRef/jabref/issues/11106)
- We fixed an issue where web search preferences "Custom API key" table modifications not discarded. [#11925](https://github.com/JabRef/jabref/issues/11925)
- We fixed an issue where trying to open a library from a failed mounted directory on Mac would cause an error. [#10548](https://github.com/JabRef/jabref/issues/10548)

### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,14 @@
import org.jabref.logic.shared.exception.InvalidDBMSConnectionPropertiesException;
import org.jabref.logic.shared.exception.NotASharedDatabaseException;
import org.jabref.logic.util.BackgroundTask;
import org.jabref.logic.util.Directories;
import org.jabref.logic.util.StandardFileType;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.logic.util.io.FileHistory;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.util.FileUpdateMonitor;

import com.google.common.annotations.VisibleForTesting;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -102,20 +104,47 @@ public static void performPostOpenActions(ParserResult result, DialogService dia

@Override
public void execute() {
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
List<Path> filesToOpen = getFilesToOpen();
openFiles(new ArrayList<>(filesToOpen));
}

@VisibleForTesting
List<Path> getFilesToOpen() {
List<Path> filesToOpen;

try {
FileDialogConfiguration initialDirectoryConfig = getFileDialogConfiguration(getInitialDirectory());
filesToOpen = dialogService.showFileOpenDialogAndGetMultipleFiles(initialDirectoryConfig);
} catch (IllegalArgumentException e) {
// See https://github.com/JabRef/jabref/issues/10548 for details
// Rebuild a new config with the home directory
FileDialogConfiguration homeDirectoryConfig = getFileDialogConfiguration(Directories.getUserDirectory());
filesToOpen = dialogService.showFileOpenDialogAndGetMultipleFiles(homeDirectoryConfig);
}

return filesToOpen;
}

/**
* Builds a new FileDialogConfiguration using the given path as the initial directory for use in
* dialogService.showFileOpenDialogAndGetMultipleFiles().
*
* @param initialDirectory Path to use as the initial directory
* @return new FileDialogConfig with given initial directory
*/
public FileDialogConfiguration getFileDialogConfiguration(Path initialDirectory) {
return new FileDialogConfiguration.Builder()
.addExtensionFilter(StandardFileType.BIBTEX_DB)
.withDefaultExtension(StandardFileType.BIBTEX_DB)
.withInitialDirectory(getInitialDirectory())
.withInitialDirectory(initialDirectory)
.build();

List<Path> filesToOpen = dialogService.showFileOpenDialogAndGetMultipleFiles(fileDialogConfiguration);
openFiles(new ArrayList<>(filesToOpen));
}

/**
* @return Path of current panel database directory or the working directory
*/
private Path getInitialDirectory() {
@VisibleForTesting
Path getInitialDirectory() {
if (tabContainer.getLibraryTabs().isEmpty()) {
return preferences.getFilePreferences().getWorkingDirectory();
} else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package org.jabref.gui.importer.actions;

import java.nio.file.Path;
import java.util.List;

import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.DialogService;
import org.jabref.gui.LibraryTabContainer;
import org.jabref.gui.StateManager;
import org.jabref.gui.preferences.GuiPreferences;
import org.jabref.gui.undo.CountingUndoManager;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.FilePreferences;
import org.jabref.logic.ai.AiService;
import org.jabref.logic.util.Directories;
import org.jabref.logic.util.TaskExecutor;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.util.FileUpdateMonitor;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

public class OpenDatabaseActionTest {
DialogService dialogService;
GuiPreferences guiPreferences;
OpenDatabaseAction openDatabaseAction;

@BeforeEach
void initializeOpenDatabaseAction() {
dialogService = mock(DialogService.class);
guiPreferences = mock(GuiPreferences.class);
openDatabaseAction = spy(new OpenDatabaseAction(
mock(LibraryTabContainer.class),
guiPreferences,
mock(AiService.class),
dialogService,
mock(StateManager.class),
mock(FileUpdateMonitor.class),
mock(BibEntryTypesManager.class),
mock(CountingUndoManager.class),
mock(ClipBoardManager.class),
mock(TaskExecutor.class)
));
}

@Test
void getFilesToOpenFailsToOpenPath(@TempDir Path tempDir) {
Path path = tempDir.resolve("test-dir");

FilePreferences filePreferences = mock(FilePreferences.class);
FileDialogConfiguration badConfig = mock(FileDialogConfiguration.class);
FileDialogConfiguration goodConfig = mock(FileDialogConfiguration.class);

when(guiPreferences.getFilePreferences()).thenReturn(filePreferences);
when(openDatabaseAction.getInitialDirectory()).thenReturn(path);

// Make it so that showFileOpenDialogAndGetMultipleFiles will throw an error when called with the bad path, but
// not for the good path as in issue #10548
when(dialogService.showFileOpenDialogAndGetMultipleFiles(badConfig))
.thenAnswer(x -> {
throw new IllegalArgumentException();
});
when(dialogService.showFileOpenDialogAndGetMultipleFiles(goodConfig))
.thenAnswer(x -> List.of());

// Simulate a scenario where the initial directory is good
when(openDatabaseAction.getFileDialogConfiguration(openDatabaseAction.getInitialDirectory()))
.thenReturn(goodConfig);

assertEquals(List.of(), openDatabaseAction.getFilesToOpen());

// Simulate a scenario where the initial directory is bad and the user directory is good
when(openDatabaseAction.getFileDialogConfiguration(openDatabaseAction.getInitialDirectory()))
.thenReturn(badConfig);
when(openDatabaseAction.getFileDialogConfiguration(Directories.getUserDirectory()))
.thenReturn(goodConfig);

assertThrows(IllegalArgumentException.class, () -> dialogService.showFileOpenDialogAndGetMultipleFiles(badConfig));
assertDoesNotThrow(() -> dialogService.showFileOpenDialog(goodConfig));
assertEquals(List.of(), openDatabaseAction.getFilesToOpen());
}
}

0 comments on commit 60e9ded

Please sign in to comment.