jabref icon indicating copy to clipboard operation
jabref copied to clipboard

New Entry: If DOI already exists in library offer "jump to entry"

Open koppor opened this issue 10 months ago • 7 comments

  1. Create a new bib file
  2. Add new entry for https://doi.org/10.1016/j.cl.2018.01.004
    Image
  3. Add new entry for https://doi.org/10.1016/j.cl.2018.01.004

See that dialog is the same:

Image

Wish: Below the DOI identifier there is a hint displayed:

"Entry already existing in library. Select entry."

"Select entry." is a Hyperlink. It selectes the entry in the respective library.

Hints:

  • org.jabref.gui.LibraryTab#showAndEditcan be used to jump to the entry

Next step

After the initial functionality is made, one can implement it to handle more than the currently opened tab

See org.jabref.gui.search.GlobalSearchResultDialog#initialize at resultsTable.setOnMouseClicked(event -> { how to search an entry in any tab.

The hint should say: "Entry found in library X. Select entry."

koppor avatar Jun 05 '25 07:06 koppor

/assign-me

jayvardhanghildiyal avatar Jun 05 '25 08:06 jayvardhanghildiyal

👋 Hey @jayvardhanghildiyal, thank you for your interest in this issue! 🎉

We're excited to have you on board. Start by exploring our Contributing guidelines, and don't forget to check out our workspace setup guidelines to get started smoothly.

For questions on JabRef functionality and the code base, you can consult the JabRef Guru or ask on our Gitter chat.

In case you encounter failing tests during development, please check our developer FAQs!

Having any questions or issues? Feel free to ask here on GitHub. Need help setting up your local workspace? Join the conversation on JabRef's Gitter chat. And don't hesitate to open a (draft) pull request early on to show the direction it is heading towards. This way, you will receive valuable feedback.

Happy coding! 🚀

github-actions[bot] avatar Jun 05 '25 08:06 github-actions[bot]

Hello ! It's the end of my semester at university and I've been busy with my final exams amidst getting familiar with JabRef.

I'd really like to continue working on this issue and complete it. Can I keep working on it without it being unassigned ?

jayvardhanghildiyal avatar Jun 10 '25 12:06 jayvardhanghildiyal

@jayvardhanghildiyal I added the "Pinned" label and hope for the best 😅

koppor avatar Jun 16 '25 13:06 koppor

Hello again ! I am really really sorry for my lack of communication. My exams and university projects are finally over. I don't know where the time went. But since the weekend, I've been giving this issue my full attention. I'd like to show two small things that I've done.

Image

I've added an HBox in the NewEntry.fxml file. I will work on changing it's visibility and manage property. With that, it shouldn't take any extra space when hidden. I hope the position of the warning is appropriate. If not, please let me know !

<HBox fx:id="duplicateWarningBox" alignment="CENTER_LEFT" spacing="5.0" visible="true" managed="true">
           <Label text="Entry already existing in library."/>
          <Hyperlink fx:id="selectEntryLink" text="Select entry"/>
</HBox>

I was looking into how I can access the data from the entries or get access to the .bib file, but realized duplicate checking is already implemented. I found a file named DuplicateCheck.java and hope I can find what I need there.

I would like to apologize again. In my next comment, hopefully I can show you more.

jayvardhanghildiyal avatar Jun 16 '25 15:06 jayvardhanghildiyal

I was looking into how I can access the data from the entries or get access to the .bib file, but realized duplicate checking is already implemented. I found a file named DuplicateCheck.java and hope I can find what I need there.

It's more simpler -- just query all existing (!) entrie for the field DOI and check if the contents are equal. OK, maybe you have to create a DOI object to ensure that the different "variations" of DOI display work.

Think, you need to use the StateManager to access all opened libries. But, start from the active tab. if then not found, ask the other tabs. - When you found something, you can ask the tab for selectEntry (or similarily named)

koppor avatar Jun 17 '25 00:06 koppor

That is much simpler ! Thank you for the help. I'll show you what I have done so far.

Using the StateManager, I was able to access all entries. A listener calls the checkDOI function everytime the text in the textfield gets updated.

....
        idText.textProperty().addListener((obs, oldText, newText) -> {
            checkDOI(newText);
        })
....

There is another function that I wrote, named checkDOI. I found another useful method called doiStrip to strip all prefixes from the DOI Identifier entered in the textfield, making DOI entry comparison possible.

....
        LayoutFormatter doiStrip = new DOIStrip();

        for (BibEntry entry : entries) {
            entry.getField(StandardField.DOI).ifPresent(existingDoi -> {
                if (existingDoi.equalsIgnoreCase(doiStrip.format(doiInput))) {
                    System.out.println("Duplicate DOI found: " + doiInput);
                }
            });
        }
....

I am logging all outputs in the terminal for now, so that I can monitor if something goes wrong.

Image

Now that I am able to detect duplicates, I'll work on wiring this code to the visible and manage property of my HBox shown earlier.

Edit:

I'll work on wiring this code to the visible and manage property of my HBox shown earlier.

I have implemented this part ! I tried uploading a video as a demonstration but it would not let me. I'll continue my work on the final part, directing the user to the library where the file can be found using the hyperlink !

jayvardhanghildiyal avatar Jun 18 '25 13:06 jayvardhanghildiyal

(on my mobile; excuse brevity)

  • The bibentrys doi should alsl be stripped
  • use stream().map().filter().findfirst.map()
  • in the first map, map to a tuple bibentry and strippeddoi; the doi is then used at filter. last map returns the bibentry

koppor avatar Jun 19 '25 13:06 koppor

Edit : I just saw your earlier comment after posting this comment. I'll look into applying the information provided. I really hope I can complete this issue by Sunday !

Hey there ! I was trying to modify my code to try fit the next step that is required to proceed (searching for the entries in all active tabs/libraries, not just the active tab). I used the hint provided to the GlobalSearchResultDialog.java, but I am having a few problems.

In the code below, I am trying to loop through every tab and select it's entries. Then, we loop through those entries. The rest of the logic is mostly the same as before.

private void checkDOI(String doiInput) {
        
....

        for (LibraryTab tab : libraryTabContainer.getLibraryTabs()) {
            BibDatabaseContext context = tab.getBibDatabaseContext();
            for (BibEntry entry : context.getEntries()) {
                Optional<String> doi = entry.getField(StandardField.DOI);
                if (doi.isPresent() && doi.get().equalsIgnoreCase(normalizedInput)) {
                    System.out.println("Duplicate DOI found in library: " + context.getDatabasePath().orElse(Path.of("Unnamed")));

                    // set the view model property so label shows
                    viewModel.isDuplicateEntryProperty().set(true);
                    duplicateEntry = entry;

                    // focus the matching tab
                    libraryTabContainer.showLibraryTab(tab);

                    return;
                }
            }
        }
        // if no duplicates are found
        viewModel.isDuplicateEntryProperty().set(false);
}

I've also imported the class and created a variable.

import org.jabref.gui.LibraryTabContainer;
private final LibraryTabContainer libraryTabContainer;

The problem is that I have to initialize the libraryTabContainer object. And I don't really understand how to do that. I knew that I didn't really give the libraryTabContainer any value so the code was not going to work.

Initially, it looked like I have to pass the libraryTabContainer object through the NewEntryView.java constructor. Modifying a class constructor is probably not a good idea, but I tried. That led me to the NewEntryAction.java file, that has three separate constructors and so on. I think that maybe this is the wrong path to go down to. I do not know how I am supposed to proceed. Any insights would be appreciated.

jayvardhanghildiyal avatar Jun 19 '25 13:06 jayvardhanghildiyal

You can pass the list of open libraries down but I think you need to put them in the state manager as they can change, e.g. user closes/opens another one

Siedlerchr avatar Jun 19 '25 14:06 Siedlerchr

You can pass the list of open libraries down but I think you need to put them in the state manager as they can change, e.g. user closes/opens another one

The state manager should have the list of open tabs?!

koppor avatar Jun 19 '25 15:06 koppor

It currently only has the active tab and a list of BIbDatabaseContext https://github.com/JabRef/jabref/blob/518f61f4fc13fa4aed1b750765dc0765b3bc7a90/jabgui/src/main/java/org/jabref/gui/StateManager.java#L63-L65

Siedlerchr avatar Jun 19 '25 15:06 Siedlerchr

You can pass the list of open libraries down but I think you need to put them in the state manager as they can change, e.g. user closes/opens another one

Oh hi there @Siedlerchr ! I had a question regarding this (it might be a bit silly). So when we are trying to create a new entry and when the duplicate warning appears (only for the currently active tab for now), the user can't open or close any other tabs, right ? I tried testing it out. When the dialog box appears on top, the UI behind it does not seem accessable. So, the user shouldn't be able to close or open tabs, right ?

Image

jayvardhanghildiyal avatar Jun 19 '25 15:06 jayvardhanghildiyal

Good point. Yes, the dialog is a model so the other elements are not accesible

Siedlerchr avatar Jun 19 '25 15:06 Siedlerchr

Another thing: Maybe the dois should be cached in a Map strippedDOI->BibEntry. Reason for a 50k sized library the lookup will take long if there is always an iteration.

koppor avatar Jun 20 '25 06:06 koppor

Good point. Yes, the dialog is a model so the other elements are not accesible

Then we just stick to the current library, which is OK.

koppor avatar Jun 20 '25 06:06 koppor

See org.jabref.gui.search.GlobalSearchResultDialog#initialize at resultsTable.setOnMouseClicked(event -> { how to search an entry in any tab.

Based on the hint provided, the GlobalSearchResultDialog.java file takes in the LibraryTabContainer argument in its constructor. Reading the file and all others that use the LibraryTabContainer interface, access to LibraryTabContainer is made the same way.

The NewEntryView.java constructor is used in the NewEntryAction.java file to create the view. If I were to modify the existing constructor in NewEntryView.java or create a new one (that passes the LibraryTabContainer interface) it prompts me to change definitions of a lot of functions used everywhere in the codebase.

I think you need to put them in the state manager

I read through the StateManager file and understood the different types of collections that exist. I tried creating an ObservableList and a method to fetch it.

private final ObservableList<BibDatabaseContext> openTabs = FXCollections.observableArrayList();
....
public ObservableList<BibDatabaseContext> getOpenTabs() {
        return openTabs;
}

I also noticed that these lists are updated whenever an action takes place (which makes sense). For example, whenever a new tab is created, NewDataBaseAction.java adds the tab to the container as such.

@Override
    public void execute() {
        BibDatabaseContext bibDatabaseContext = new BibDatabaseContext();
        bibDatabaseContext.setMode(preferences.getLibraryPreferences().getDefaultBibDatabaseMode());
        tabContainer.addTab(bibDatabaseContext, true);
    }

But I encounter the same problem of passing the StateManager to the NewDataBaseAction.java constructor.

I've tried studying other files that utilize this interface, but I'm still a bit stumped. I apologize for my lack of knowledge. Is there an alternate approach that I can take ? Or another method that will give me access to the LibraryTabContainer ?

jayvardhanghildiyal avatar Jun 20 '25 06:06 jayvardhanghildiyal

Oh sorry I didn't see your messages earlier. I posted my message about the same time as you apparently so I wasn't able to see it.

Another thing: Maybe the dois should be cached in a Map strippedDOI->BibEntry. Reason for a 50k sized library the lookup will take long if there is always an iteration.

Sure ! I can work on that. Fetching data from maps would definitely be faster.

Then we just stick to the current library, which is OK.

Oh I see. So does that mean my current implementation for the issue (checking for duplicates in current/active tab only) is sufficient ? If so, I can start cleaning up my code ! Completing this by Sunday would be great.

If not, do let me know what else I can do !

jayvardhanghildiyal avatar Jun 20 '25 08:06 jayvardhanghildiyal

Another thing: Maybe the dois should be cached in a Map strippedDOI->BibEntry. Reason for a 50k sized library the lookup will take long if there is always an iteration.

Hi ! We begin with a map that has a key-value pair of String and BibEntry. We also have a boolean value (we will discuss this later).

private final HashMap<String, BibEntry> doiCache = new HashMap<String, BibEntry>();
private boolean isCacheInitialized = false;

The new main logic of the checkDOI function is posted below.

We use a boolean value isCacheInitialized that is set to true / false depending on whether doiCache contains the necessary values. Iteration through all entries in a library happens only ONCE. After that, the cache is used to check for duplicates. Switching to another library empties the cache, fills it for the current library and uses it.

private void checkDOI(String doiInput) {
        
....
        //  check if a ready-to-use map already exists
        if (!isCacheInitialized) {
            doiCache.clear();
            BibDatabaseContext databaseContext = stateManager.getActiveDatabase().orElseThrow(() -> new NullPointerException("no active library found"));

            for (BibEntry entry : databaseContext.getEntries()) {
                entry.getField(StandardField.DOI)
                     .map(doiStrip::format)
                     .ifPresent(strippedDoi -> doiCache.put(strippedDoi.toLowerCase(), entry));
            }

            isCacheInitialized = true;
        }

        // use the doiCache to fetch entry in question
        BibEntry matchedEntry = doiCache.get(normalizedInput.toLowerCase());

....
    }

Let me know what you guys think about this implementation !

jayvardhanghildiyal avatar Jun 20 '25 14:06 jayvardhanghildiyal

Oh I see. So does that mean my current implementation for the issue (checking for duplicates in current/active tab only) is sufficient ? If so, I can start cleaning up my code !

I guess everything is good and well ! I'll go through the documentation and Code-Howtos to see if i missed anything required prior to making a pull request. My issue is based on UI, so according to the contribution document, it seems safe to assume that I should not have to make test cases.

After that, I'll make a pull request !

jayvardhanghildiyal avatar Jun 21 '25 12:06 jayvardhanghildiyal

Go ahead with a PR that makes it easier for us to see the code in context and to comment on it

Siedlerchr avatar Jun 21 '25 12:06 Siedlerchr

We think that this issue was fixed. Please head to https://builds.jabref.org/main to download a development build and try it out.

For any feedback, add a comment to the pull request at https://github.com/JabRef/jabref/pull/13390.

github-actions[bot] avatar Jul 04 '25 16:07 github-actions[bot]