Fix BibTeX "twin" at citation relations (#12124)

* Add cloning of BibEntries while adding

- Add getter+setter for "cites" to BibEntry
- Refactor CitationsRelationsTabViewModel
- Group citation keys together in JabRef_en.properties
- We fixed an issue where the import of an entry cited by an entry caused a change of the citation key of the citing entry.

* Reorder JabRef_en.properties

* Merge same language strings into another string

* Exit early

* Convert everything to a parameter

* use "entry(s)" instead of "entry(ies)"

* The tests are BibTeX (not biblatex)

* Line breaks...

* Fix test "importedEntriesWithExistingCitationKeysCiteExistingEntry"

* Fix localization

* Always select entry in main table when showing entry editor

* Revert "Always select entry in main table when showing entry editor"

This reverts commit d618728.

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

@@ -526,7 +526,7 @@ private void importEntries(List<CitationRelationItem> entriesToImport, CitationF
 
         citationsRelationsTabViewModel.importEntries(entriesToImport, searchType, existingEntry);
 
-        dialogService.notify(Localization.lang("Number of entries successfully imported") + ": " + entriesToImport.size());
+        dialogService.notify(Localization.lang("%0 entry(s) imported", entriesToImport.size()));
     }
 
     /**

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModel.java

@@ -1,8 +1,7 @@
 package org.jabref.gui.entryeditor.citationrelationtab;
 
-import java.util.Arrays;
 import java.util.List;
-import java.util.stream.Collectors;
+import java.util.SequencedSet;
 
 import javax.swing.undo.UndoManager;
 
@@ -12,11 +11,10 @@
 import org.jabref.gui.externalfiles.ImportHandler;
 import org.jabref.gui.preferences.GuiPreferences;
 import org.jabref.logic.citationkeypattern.CitationKeyGenerator;
-import org.jabref.logic.citationkeypattern.CitationKeyPatternPreferences;
+import org.jabref.logic.l10n.Localization;
 import org.jabref.logic.util.TaskExecutor;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
-import org.jabref.model.entry.field.StandardField;
 import org.jabref.model.util.FileUpdateMonitor;
 
 public class CitationsRelationsTabViewModel {
@@ -40,7 +38,11 @@ public CitationsRelationsTabViewModel(BibDatabaseContext databaseContext, GuiPre
     }
 
     public void importEntries(List<CitationRelationItem> entriesToImport, CitationFetcher.SearchType searchType, BibEntry existingEntry) {
-        List<BibEntry> entries = entriesToImport.stream().map(CitationRelationItem::entry).toList();
+        List<BibEntry> entries = entriesToImport.stream()
+                                                .map(CitationRelationItem::entry)
+                                                // We need to have a clone of the entry, because we add the entry to the library (and keep it in the citation relation tab, too)
+                                                .map(entry -> (BibEntry) entry.clone())
+                                                .toList();
 
         ImportHandler importHandler = new ImportHandler(
                 databaseContext,
@@ -50,67 +52,51 @@ public void importEntries(List<CitationRelationItem> entriesToImport, CitationFe
                 stateManager,
                 dialogService,
                 taskExecutor);
+        CitationKeyGenerator generator = new CitationKeyGenerator(databaseContext, preferences.getCitationKeyPatternPreferences());
+        boolean generateNewKeyOnImport = preferences.getImporterPreferences().generateNewKeyOnImportProperty().get();
 
         switch (searchType) {
-            case CITES -> importCites(entries, existingEntry, importHandler);
-            case CITED_BY -> importCitedBy(entries, existingEntry, importHandler);
+            case CITES -> importCites(entries, existingEntry, importHandler, generator, generateNewKeyOnImport);
+            case CITED_BY -> importCitedBy(entries, existingEntry, importHandler, generator, generateNewKeyOnImport);
         }
     }
 
-    private void importCites(List<BibEntry> entries, BibEntry existingEntry, ImportHandler importHandler) {
-        CitationKeyPatternPreferences citationKeyPatternPreferences = preferences.getCitationKeyPatternPreferences();
-        CitationKeyGenerator generator = new CitationKeyGenerator(databaseContext, citationKeyPatternPreferences);
-        boolean generateNewKeyOnImport = preferences.getImporterPreferences().generateNewKeyOnImportProperty().get();
-
-        List<String> citeKeys = getExistingEntriesFromCiteField(existingEntry);
-        citeKeys.removeIf(String::isEmpty);
+    private void importCites(List<BibEntry> entries, BibEntry existingEntry, ImportHandler importHandler, CitationKeyGenerator generator, boolean generateNewKeyOnImport) {
+        SequencedSet<String> citeKeys = existingEntry.getCites();
 
         for (BibEntry entryToCite : entries) {
             if (generateNewKeyOnImport || entryToCite.getCitationKey().isEmpty()) {
                 String key = generator.generateKey(entryToCite);
                 entryToCite.setCitationKey(key);
-                addToKeyToList(citeKeys, key);
-            } else {
-                addToKeyToList(citeKeys, entryToCite.getCitationKey().get());
             }
+            citeKeys.add(entryToCite.getCitationKey().get());
         }
-        existingEntry.setField(StandardField.CITES, toCommaSeparatedString(citeKeys));
+
+        existingEntry.setCites(citeKeys);
         importHandler.importEntries(entries);
     }
 
-    private void importCitedBy(List<BibEntry> entries, BibEntry existingEntry, ImportHandler importHandler) {
-        CitationKeyPatternPreferences citationKeyPatternPreferences = preferences.getCitationKeyPatternPreferences();
-        CitationKeyGenerator generator = new CitationKeyGenerator(databaseContext, citationKeyPatternPreferences);
-        boolean generateNewKeyOnImport = preferences.getImporterPreferences().generateNewKeyOnImportProperty().get();
-
-        for (BibEntry entryThatCitesOurExistingEntry : entries) {
-            List<String> existingCites = getExistingEntriesFromCiteField(entryThatCitesOurExistingEntry);
-            existingCites.removeIf(String::isEmpty);
-            String key;
-            if (generateNewKeyOnImport || entryThatCitesOurExistingEntry.getCitationKey().isEmpty()) {
-                key = generator.generateKey(entryThatCitesOurExistingEntry);
-                entryThatCitesOurExistingEntry.setCitationKey(key);
-            } else {
-                key = existingEntry.getCitationKey().get();
+    /**
+     * "cited by" is the opposite of "cites", but not stored in field `CITED_BY`, but in the `CITES` field of the citing entry.
+     * <p>
+     * Therefore, some special handling is needed
+     */
+    private void importCitedBy(List<BibEntry> entries, BibEntry existingEntry, ImportHandler importHandler, CitationKeyGenerator generator, boolean generateNewKeyOnImport) {
+        if (existingEntry.getCitationKey().isEmpty()) {
+            if (!generateNewKeyOnImport) {
+                dialogService.notify(Localization.lang("No citation key for %0", existingEntry.getAuthorTitleYear()));
+                return;
             }
-            addToKeyToList(existingCites, key);
-            entryThatCitesOurExistingEntry.setField(StandardField.CITES, toCommaSeparatedString(existingCites));
+            existingEntry.setCitationKey(generator.generateKey(existingEntry));
         }
+        String citationKey = existingEntry.getCitationKey().get();
 
-        importHandler.importEntries(entries);
-    }
-
-    private void addToKeyToList(List<String> list, String key) {
-        if (!list.contains(key)) {
-            list.add(key);
+        for (BibEntry citingEntry : entries) {
+            SequencedSet<String> existingCites = citingEntry.getCites();
+            existingCites.add(citationKey);
+            citingEntry.setCites(existingCites);
         }
-    }
 
-    private List<String> getExistingEntriesFromCiteField(BibEntry entry) {
-        return Arrays.stream(entry.getField(StandardField.CITES).orElse("").split(",")).collect(Collectors.toList());
-    }
-
-    private String toCommaSeparatedString(List<String> citeentries) {
-        return String.join(",", citeentries);
+        importHandler.importEntries(entries);
     }
 }

src/main/java/org/jabref/gui/importer/ImportCommand.java

@@ -109,7 +109,7 @@ private void importSingleFile(Path file, SortedSet<Importer> importers, FileChoo
         if (importMethod == ImportMethod.AS_NEW) {
             task.onSuccess(parserResult -> {
                     tabContainer.addTab(parserResult.getDatabaseContext(), true);
-                    dialogService.notify(Localization.lang("Imported entries") + ": " + parserResult.getDatabase().getEntries().size());
+                    dialogService.notify(Localization.lang("%0 entry(s) imported", parserResult.getDatabase().getEntries().size()));
                 })
                 .onFailure(ex -> {
                     LOGGER.error("Error importing", ex);

src/main/java/org/jabref/model/entry/BibEntry.java

@@ -1,6 +1,7 @@
 package org.jabref.model.entry;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -18,6 +19,7 @@
 import java.util.function.BiFunction;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javafx.beans.Observable;
 import javafx.beans.property.ObjectProperty;
@@ -1124,6 +1126,26 @@ public Optional<FieldChange> addFiles(List<LinkedFile> filesToAdd) {
     }
     // endregion
 
+    /**
+     * Checks {@link StandardField#CITES} for a list of citation keys and returns them.
+     * <p>
+     * Empty citation keys are not returned. There is no consistency check made.
+     *
+     * @return List of citation keys; empty list if field is empty or not available.
+     */
+    public SequencedSet<String> getCites() {
+        return this.getField(StandardField.CITES)
+                   .map(content -> Arrays.stream(content.split(",")))
+                   .orElseGet(Stream::empty)
+                   .map(String::trim)
+                   .filter(key -> !key.isEmpty())
+                   .collect(Collectors.toCollection(LinkedHashSet::new));
+    }
+
+    public Optional<FieldChange> setCites(SequencedSet<String> keys) {
+        return this.setField(StandardField.CITES, keys.stream().collect(Collectors.joining(",")));
+    }
+
     public void setDate(Date date) {
         date.getYear().ifPresent(year -> setField(StandardField.YEAR, year.toString()));
         date.getMonth().ifPresent(this::setMonth);

src/main/resources/l10n/JabRef_en.properties

@@ -481,8 +481,6 @@ Import\ preferences=Import preferences
 
 Import\ preferences\ from\ file=Import preferences from file
 
-Imported\ entries=Imported entries
-
 Importer\ class=Importer class
 
 Include\ subgroups\:\ When\ selected,\ view\ entries\ contained\ in\ this\ group\ or\ its\ subgroups=Include subgroups: When selected, view entries contained in this group or its subgroups
@@ -1007,7 +1005,8 @@ Error\ opening\ file=Error opening file
 Error\ opening\ file\ '%0'=Error opening file '%0'
 File\ '%0'\ already\ linked=File '%0' already linked
 
-Number\ of\ entries\ successfully\ imported=Number of entries successfully imported
+%0\ entry(s)\ imported=%0 entry(s) imported
+
 Error\ while\ fetching\ from\ %0=Error while fetching from %0
 
 Unable\ to\ open\ link.=Unable to open link.
@@ -1761,6 +1760,8 @@ Remote\ services=Remote services
 Cannot\ use\ port\ %0\ for\ remote\ operation;\ another\ application\ may\ be\ using\ it.\ Try\ specifying\ another\ port.=Cannot use port %0 for remote operation; another application may be using it. Try specifying another port.
 Grobid\ URL=Grobid URL
 Allow\ sending\ PDF\ files\ and\ raw\ citation\ strings\ to\ a\ JabRef\ online\ service\ (Grobid)\ to\ determine\ Metadata.\ This\ produces\ better\ results.=Allow sending PDF files and raw citation strings to a JabRef online service (Grobid) to determine Metadata. This produces better results.
+Send\ to\ Grobid=Send to Grobid
+Do\ not\ send=Do not send
 
 Proxy\ requires\ password=Proxy requires password
 Proxy\ configuration=Proxy configuration
@@ -2809,6 +2810,3 @@ Citation\ Entry=Citation Entry
 
 File\ Move\ Errors=File Move Errors
 Could\ not\ move\ file\ %0.\ Please\ close\ this\ file\ and\ retry.=Could not move file %0. Please close this file and retry.
-
-Send\ to\ Grobid=Send to Grobid
-Do\ not\ send=Do not send

src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java

@@ -24,6 +24,7 @@
 import org.jabref.logic.util.CurrentThreadTaskExecutor;
 import org.jabref.model.database.BibDatabase;
 import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.database.BibDatabaseMode;
 import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.field.StandardField;
 import org.jabref.model.entry.types.StandardEntryType;
@@ -79,6 +80,7 @@ void setUp() {
         when(preferences.getCitationKeyPatternPreferences()).thenReturn(citationKeyPatternPreferences);
 
         bibDatabaseContext = new BibDatabaseContext(new BibDatabase());
+        bibDatabaseContext.setMode(BibDatabaseMode.BIBTEX);
         when(duplicateCheck.isDuplicate(any(), any(), any())).thenReturn(false);
 
         StateManager stateManager = mock(StateManager.class, Answers.RETURNS_DEEP_STUBS);
@@ -109,31 +111,42 @@ void setUp() {
 
     @Test
     void existingEntryCitesOtherPaperWithCitationKeys() {
-        var citationItems = List.of(new CitationRelationItem(firstEntryToImport, false),
+        var citationItems = List.of(
+                new CitationRelationItem(firstEntryToImport, false),
                 new CitationRelationItem(secondEntryToImport, false));
 
         viewModel.importEntries(citationItems, CitationFetcher.SearchType.CITES, existingEntry);
+
         assertEquals(Optional.of("FirstAuthorCitationKey2022,SecondAuthorCitationKey20221"), existingEntry.getField(StandardField.CITES));
         assertEquals(List.of(existingEntry, firstEntryToImport, secondEntryToImport), bibDatabaseContext.getEntries());
     }
 
     @Test
     void importedEntriesWithExistingCitationKeysCiteExistingEntry() {
-        var citationItems = List.of(new CitationRelationItem(firstEntryToImport, false),
+        var citationItems = List.of(
+                new CitationRelationItem(firstEntryToImport, false),
                 new CitationRelationItem(secondEntryToImport, false));
 
         viewModel.importEntries(citationItems, CitationFetcher.SearchType.CITED_BY, existingEntry);
-        assertEquals(Optional.of("Test2023"), firstEntryToImport.getField(StandardField.CITES));
-        assertEquals(List.of(existingEntry, firstEntryToImport, secondEntryToImport), bibDatabaseContext.getEntries());
+
+        // The entries are cloned during the import. Thus, we need to get the actual entries from the database.
+        // In the test, the citation key is not changed during the import, thus we can just look up the entries by their citation key.
+        BibEntry firstEntryInLibrary = bibDatabaseContext.getDatabase().getEntryByCitationKey(firstEntryToImport.getCitationKey().get()).get();
+        BibEntry secondEntryInLibrary = bibDatabaseContext.getDatabase().getEntryByCitationKey(secondEntryToImport.getCitationKey().get()).get();
+
+        assertEquals(Optional.of("Test2023"), firstEntryInLibrary.getField(StandardField.CITES));
+        assertEquals(List.of(existingEntry, firstEntryInLibrary, secondEntryInLibrary), bibDatabaseContext.getEntries());
     }
 
     @Test
     void existingEntryCitesOtherPaperWithCitationKeysAndExistingCiteField() {
         existingEntry.setField(StandardField.CITES, "Asdf1222");
-        var citationItems = List.of(new CitationRelationItem(firstEntryToImport, false),
+        var citationItems = List.of(
+                new CitationRelationItem(firstEntryToImport, false),
                 new CitationRelationItem(secondEntryToImport, false));
 
         viewModel.importEntries(citationItems, CitationFetcher.SearchType.CITES, existingEntry);
+
         assertEquals(Optional.of("Asdf1222,FirstAuthorCitationKey2022,SecondAuthorCitationKey20221"), existingEntry.getField(StandardField.CITES));
         assertEquals(List.of(existingEntry, firstEntryToImport, secondEntryToImport), bibDatabaseContext.getEntries());
     }