[WIP] This relativizes the PDF's filepaths after importing through "Find Unlinked Files"

[IceMajor2]

Fixes https://github.com/koppor/jabref/issues/549

I'll start explaining the changes I've made from the top of the method chain.

1. ImportHandler It is the place where the logic of importing a PDF file starts being executed. At line 119, there's an importPDFContent (of ExternalFilesContentImporter class) method executed, which takes in a filepath as a parameter. In order to access the "General file directory" library property, I've modified this method (see 2nd point).
2. ExternalFilesContentImporter As mentioned, importPDFContent method gets a new parameter: an instance of MetaData. The "General file directory" path is stored there.
3. PdfMergeMetadataImporter Finally, the actual logic is being executed here. The class is an implementation of an abstract class Importer. Among others, it overrides importDatabase(Path) method. Previous step's method importPDFContent calls PdfMergeMetadataImporter's importDatabase method, where the filepath can be manipulated. Because of extending that method, I can provide importDatabase with MetaData object. At last - just at the end of the method - I've included some basic logic that:

• checks whether the "General file directory" is present
• if it is, the filepath is relativized (`C
• if it is not, the absolute path is displayed

Please, do provide me with feedback on the above implementation. I'm particularly interested whether the extensions with MetaData instance is an okay decision. I'm also wondering if the "General file directory" property may be accessed in some other way.

Note that I've not included any changes to the CHANGELOG.md as this is still a work-in-progress pull request and it does not yet solve the issue of other than PDF files.

Mandatory checks

• [ ] Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
• [ ] Tests created for changes (if applicable)
• [x] Manually tested changed features in running JabRef (always required)
• [ ] Screenshots added in PR description (for UI changes)
• [x] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
• [x] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

[Siedlerchr]

Looks good so far

[IceMajor2]

Please add (at least) one test case.

You can find one at the description "How to reproduce" at koppor#549.

I've committed a new test case.

I also wanted to add a more exhaustive test by testing not only the low-level PdfMergeMetadataImporter, but also the ImportHandler. However, I ran into some issues.

In ImportHandlerTest I've written something like this:

    @Test
    void handlePathRelativizationOnImportTest() throws Exception {
        // Arrange
        Path file = Path.of(ImportHandler.class.getResource("/pdfs/example.pdf").toURI());
        Path workingDir = Path.of(ImportHandler.class.getResource("/pdfs/").toURI());

        FilePreferences filePreferences = preferencesService.getFilePreferences();
        when(bibDatabaseContext.getFirstExistingFileDir(filePreferences)).thenReturn(Optional.of(workingDir));

        List<ImportFilesResultItemViewModel> resultList = new ArrayList<>();

        // Act
        BackgroundTask<List<ImportFilesResultItemViewModel>> importFilesInBackground = importHandler
                .importFilesInBackground(Collections.singletonList(file))
                .onSuccess(resultList::addAll);
        importFilesInBackground.executeWith(new CurrentThreadTaskExecutor());

        // Assert
        // ...
    }

But I'd end up with Illegal State Exception: Toolkit not initialized. Then, I tried going even one more level above by testing the UnlinkedFilesDialogViewModel's startImport method, but had trouble to do so, too.

I'd like to continue to work on the test case in ImportHandler, but I need help on how to run the BackgroundTask in test environment.

[koppor]

I'd like to continue to work on the test case in ImportHandler, but I need help on how to run the BackgroundTask in test environment.

Avoid the background task in tests. Isn't there another method you can check directly?

You would run a UI test then. Search for @GUITest annotations and read how these tests are run.

You changed logic, thus you should have a test in org.jabref.logic. Because your thing should be independent from the UI. I think, tests are in PdfMergeMetadataImporterTest?

[IceMajor2]

Avoid the background task in tests. Isn't there another method you can check directly?

I was talking here about a "high-level" method - at the top of the method chain. In this draft PR, I've so far modified 3 main classes: ImportHandler, ExternalFilesContentImporter and PdfMergeMetadataImporter.

I tested the PdfMergeMetadataImporter's importDatabase(Path, Path) method that I've created (by extending with another Path parameter). So, yes, there's a method that I can check directly: I've tested it already. However, I wanted to test ImportHandler's importFilesInBackground(List<Path>) method, because it's the place where all the logic that I've implemented resides. Maybe there's still some other method that I can't find.

I can't figure out how to test it, though. It's a BackgroundTask and, going by your advice, I was unable to create a @GUITest as I can't get the "unlinked files" dialog menu working. I'm not sure how it should be initialized... I tried looking at other @GUITest tests, but couldn't manage to use those approaches in my test.

---

Anyway, please see new commits. I've implemented your suggestions and wrote a bit of JavaDoc.

[koppor]

@IceMajor2 I saw no activity the last weeks, thus I am closing. In case you are going to resume work, feel free to reopen or file a new PR.