Relink moved files [Find Unlinked Files part] #13842

paudelritij · 2025-09-09T03:01:57Z

Closes #9798

This PR relinks files, if it has been moved by user.

Steps to test

Open `test-support\src\manual-tests\issue-9798\issue-9798.bib in JabRef
Start "Lookup" -> "Find unlinked files"
Start search
File will be relinked - and following shown in the message coloumn (under import tab): File relinked to entry minimal.pdf

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
I manually tested my changes in running JabRef (always required)
I added JUnit tests for changes (if applicable)
I added screenshots in the PR description (if change is visible to the user)
I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
I checked the user 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 updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

jabref-machine · 2025-09-09T03:03:03Z

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of [x] (done), [ ] (not done yet) or [/] (not applicable).

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

paudelritij · 2025-09-24T18:13:13Z

I starting using existence code from AutoSetFileLinksUtil.java but found it could increase time complexity by using findAssociatedNotLinkedFiles() since it makes unnecessary call (in context of this feature) to while adding items in result (by calling findAssociatedFiles & findByBrokenLinkName), so I decided to create a separate method; and easy for testing as well.
org.jabref.gui.externalfiles.AutoSetFileLinksUtil#findAssociatedNotLinkedFiles was adding same link two times so I suggest to make use of set instead of list.
I had a file (a.pdf) in two places (dir/a.pdf & dir/sub-dir/a.pdf) however org.jabref.gui.externalfiles.AutoSetFileLinksUtil#findByBrokenLinkName was showing me file from sub-dir/a.pdf only so I suggest to show all related files having wantedbasename(forEach()). (one more point is that if b.txt link has been broken and findByBrokenLinkName() found file with wantedbasename to be as of b.pdf and then using code for findfirst() will add only b.pdf not b.txt which is the required one)

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

Siedlerchr · 2025-09-24T18:35:55Z

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

+                List<LinkedFile> filesToAdd = new ArrayList<>();
+
+                for (LinkedFile brokenLink : currentFiles) {
+                    if (brokenLink.findIn(bibDatabase, preferences.getFilePreferences()).isPresent()) {


Isn't this just duplicated? You add all those to entriesWithBrokenLInks that are not present so why checking them again here?

not all.. only entries having at least one broken link... though it could be improved thnks for letting know

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

trag-bot · 2025-09-25T06:00:25Z

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

+    @Test
+    void fixBrokenLinksInLargeDatabase(@TempDir Path tempDir) throws IOException {
+        Path testRoot = tempDir.resolve("test");
+        List<BibEntry> entries = new ArrayList<>();


Use modern Java data structures. Instead of using new ArrayList<>(), use List.of() for creating an empty list.

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

jabref-machine · 2025-09-25T15:44:31Z

Your pull request needs to link an issue correctly.

To ease organizational workflows, please link this pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue:

Linking a pull request to an issue using a keyword

You can link a pull request to an issue by using a supported keyword in the pull request's description or in a commit message.

Examples

✅ Fixes #xyz links pull-request to issue. Merging the PR will close the issue.
✅ Fixes https://github.com/JabRef/jabref/issues/xyz links pull-request to issue. Merging the PR will close the issue.
✅ Fixes https://github.com/Koppor/jabref/issues/xyz links pull-request to issue. Merging the PR will close the issue.
❌ Fixes [#xyz](https://github.com/JabRef/jabref/issues/xyz) links pull-request to issue. Merging the PR will NOT close the issue.

trag-bot · 2025-09-25T15:46:53Z

@trag-bot didn't find any issues in the code! ✅✨

calixtus · 2025-10-09T21:39:31Z

Is this PR ready for review?

paudelritij · 2025-10-13T01:23:38Z

Is this PR ready for review?

I did changes could anyone please review it? Thanks.

calixtus · 2025-10-13T13:35:28Z

Great, we will look into it.

jabref-machine · 2025-11-19T22:49:30Z

Your code currently does not meet JabRef's code guidelines. We use OpenRewrite to ensure "modern" Java coding practices. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Source Code Tests / OpenRewrite (pull_request)" and click on it.

The issues found can be automatically fixed. Please execute the gradle task rewriteRun from the rewrite group of the Gradle Tool window in IntelliJ, then check the results, commit, and push.

koppor

Thank you for working on this - and your patience.

Attached some comments 😅

koppor · 2025-11-20T09:25:04Z

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

+        // Verify specific file links were updated to correct relative paths
+        assertEquals(Optional.of(newPdf1Location.toString()), entry1.getFiles().stream().filter(file -> file.getLink().equals(newPdf1Location.toString())).findFirst().map(LinkedFile::getLink));
+        assertEquals(Optional.of(newDocLocation.toString()), entry1.getFiles().stream().filter(file -> file.getLink().equals(newDocLocation.toString())).findFirst().map(LinkedFile::getLink));
+        assertEquals(Optional.of(newPdf2Location.toString()), entry2.getFiles().stream().filter(file -> file.getLink().equals(newPdf2Location.toString())).findFirst().map(LinkedFile::getLink));
+        assertEquals(Optional.of(newTxtLocation.toString()), entry2.getFiles().stream().filter(file -> file.getLink().equals(newTxtLocation.toString())).findFirst().map(LinkedFile::getLink));
+        assertEquals(Optional.of(newPptLocation.toString()), entry3.getFiles().stream().filter(file -> file.getLink().equals(newPptLocation.toString())).findFirst().map(LinkedFile::getLink));
+
+        // Verify that files with similar names but different extensions were not incorrectly matched
+        assertEquals(Optional.empty(), entry1.getFiles().stream().filter(file -> file.getLink().contains("research-paper.txt")).findFirst().map(LinkedFile::getLink));
+
+        assertEquals(Optional.empty(), entry2.getFiles().stream().filter(file -> file.getLink().contains("thesis.docx")).findFirst().map(LinkedFile::getLink));


Cant the whole BibDatabase be compared? - If not, please add a code comment why there are multiple assert statements.

koppor · 2025-11-20T09:26:13Z

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

+    }
+
+    @Test
+    void fixBrokenLinksInLargeDatabase(@TempDir Path tempDir) throws IOException {


This is not what the test does. The test checks the size of the result list -- Please adapt the test name

Suggested change

void fixBrokenLinksInLargeDatabase(@TempDir Path tempDir) throws IOException {

void correctCountOfUnlinkedFiles(@TempDir Path tempDir) throws IOException {

koppor · 2025-11-20T09:26:52Z

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java

+        BibEntry entry = new BibEntry(StandardEntryType.Misc);
+        LinkedFile brokenLink = new LinkedFile("", oldPath.toString(), "PDF");
+        entry.addFile(brokenLink);


It would be nice if .withFiles could be used. - also in the other tests. -- Use it with List.of(...) and do the new LinkedFile inside.

koppor · 2025-11-20T09:27:51Z

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

+    // For testing purposes
+    public int resultListSize() {


Suggested change

// For testing purposes

public int resultListSize() {

// @VisibleForTesting

int resultListSize() {

koppor · 2025-11-20T09:30:20Z

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

+                    entry.setFiles(currentFiles);
+                }
+            } catch (IOException e) {
+                LOGGER.error("Error finding associated files for entry", e);


Not sure if the data structures are consistent in all error cases. Shouldn't the try/catch be more close to the points where the exception can occur and an appropriate handling take place?

Are results consistent in all cases? Maybe yes, but that deserves a code comment.

koppor · 2025-11-20T09:30:40Z

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

+        resultList.clear();
+        resultList.addAll(results);


This needs a Java comment - why this is done.

koppor · 2025-11-20T09:35:50Z

jabgui/src/main/java/org/jabref/gui/externalfiles/AutoSetFileLinksUtil.java

-        return linkedFiles;
+        return linkedFiles.stream().toList();


This feels not OK - promising a list in the method contract, but internally a set with a random order.

Please convert the result type to Set and also adapt the callers. Then, you will see if there is a list assumption anywhere -- and if there should be an ordering done. If an ordering needs to be done, either do it here (Maybe, use SortedSet). Or do it at the place where sorting is needed.

koppor · 2025-11-20T09:39:10Z

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java

        treeRootProperty.setValue(Optional.empty());
    }

+    public void findAndFixBrokenLinks(Path searchDirectory) {


This method needs a JavaDoc comment

Maybe also the name needs to be adjusted --> fixBrokenLinksAndReturnStillBroken

Is it possible NOT to use a global variable resultList?

Finally, this should go to org.jabref.logic.util.io.FileUtil, because, it is NOT a UI thing, but a logic (which could be used by JabKit, too)

An attempt to relink files

b929b47

paudelritij added 2 commits September 24, 2025 02:20

Merge branch 'main' into fix-for-issue-9798

1375669

Refactor fixing broken links

33c1eb9

trag-bot bot reviewed Sep 23, 2025

View reviewed changes

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java Show resolved Hide resolved

paudelritij added 4 commits September 25, 2025 02:16

Add test cases

374491c

Update JabRef_en.properties

a34fa0f

Add CHANGELOG.md

9fb6ddb

Add a comprehensive directory test case

faadb71

trag-bot bot reviewed Sep 24, 2025

View reviewed changes

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java Outdated Show resolved Hide resolved

trag-bot bot reviewed Sep 24, 2025

View reviewed changes

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java Outdated Show resolved Hide resolved

paudelritij mentioned this pull request Sep 24, 2025

Update documentation for Search for unlinked local files - Tiny rewrite and replace Swing screenshots. JabRef/user-documentation#550

Open

paudelritij marked this pull request as ready for review September 24, 2025 18:15

Siedlerchr reviewed Sep 24, 2025

View reviewed changes

jabgui/src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModel.java Outdated Show resolved Hide resolved

Siedlerchr reviewed Sep 24, 2025

View reviewed changes

Siedlerchr requested changes Sep 24, 2025

View reviewed changes

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java Outdated Show resolved Hide resolved

Siedlerchr added the status: changes-required Pull requests that are not yet complete label Sep 24, 2025

paudelritij added 2 commits September 25, 2025 15:01

Optimize method of fix broken links

edfc5d7

Reformat test cases to use assertEquals

da72c9c

trag-bot bot reviewed Sep 25, 2025

View reviewed changes

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java Show resolved Hide resolved

trag-bot bot reviewed Sep 25, 2025

View reviewed changes

jabgui/src/test/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogViewModelTest.java Show resolved Hide resolved

Revert a few changes

a67e1a4

calixtus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 13, 2025

calixtus added the component: external-files label Oct 13, 2025

koppor removed the status: changes-required Pull requests that are not yet complete label Nov 11, 2025

koppor added 2 commits November 11, 2025 15:19

Merge branch 'main' into fix-for-issue-9798

10fb428

Merge branch 'main' into fix-for-issue-9798

9d3fa0e

koppor requested changes Nov 20, 2025

View reviewed changes

github-actions bot added status: changes-required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Nov 20, 2025

	void fixBrokenLinksInLargeDatabase(@TempDir Path tempDir) throws IOException {
	void correctCountOfUnlinkedFiles(@TempDir Path tempDir) throws IOException {

Uh oh!

Relink moved files [Find Unlinked Files part] #13842

Are you sure you want to change the base?

Relink moved files [Find Unlinked Files part] #13842

Conversation

paudelritij commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Steps to test

Mandatory checks

Uh oh!

jabref-machine commented Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paudelritij commented Sep 24, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

trag-bot bot Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jabref-machine commented Sep 25, 2025

Linking a pull request to an issue using a keyword

Examples

Uh oh!

trag-bot bot commented Sep 25, 2025

Uh oh!

calixtus commented Oct 9, 2025

Uh oh!

paudelritij commented Oct 13, 2025

Uh oh!

calixtus commented Oct 13, 2025

Uh oh!

jabref-machine commented Nov 19, 2025

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

paudelritij commented Sep 9, 2025 •

edited

Loading