Create groups where paper from - SLR

CHANGELOG.md

@@ -21,6 +21,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We added "All" option to the citation fetcher combo box, which queries all providers (CrossRef, OpenAlex, OpenCitations, SemanticScholar) and merges the results into a single deduplicated list.
 - We added a quick setting toggle to enable cover images download. [#15322](https://github.com/JabRef/jabref/pull/15322)
 - We now support refreshing existing CSL citations with respect to their in-text nature in the LibreOffice integration. [#15369](https://github.com/JabRef/jabref/pull/15369)
+- We added automatic source groups to SLR results and fixed group merging to preserve all source groups. [#12542](https://github.com/JabRef/jabref/issues/12542)
 
 ### Changed
 

jabgui/src/main/java/org/jabref/gui/mergeentries/BatchEntryMergeTask.java

@@ -29,18 +29,21 @@ public class BatchEntryMergeTask extends BackgroundTask<Void> {
     private final MergingIdBasedFetcher fetcher;
     private final UndoManager undoManager;
     private final NotificationService notificationService;
+    private final char keywordSeparator;
 
     private int processedEntries;
     private int successfulUpdates;
 
     public BatchEntryMergeTask(List<BibEntry> entries,
                                MergingIdBasedFetcher fetcher,
                                UndoManager undoManager,
-                               NotificationService notificationService) {
+                               NotificationService notificationService,
+                               char keywordSeparator) {
         this.entries = entries;
         this.fetcher = fetcher;
         this.undoManager = undoManager;
         this.notificationService = notificationService;
+        this.keywordSeparator = keywordSeparator;
 
         this.compoundEdit = new NamedCompoundEdit(Localization.lang("Merge entries"));
         this.processedEntries = 0;
@@ -107,7 +110,7 @@ private Optional<String> processSingleEntry(BibEntry entry) {
         return fetcher.fetchEntry(entry)
                       .filter(MergingIdBasedFetcher.FetcherResult::hasChanges)
                       .flatMap(result -> {
-                          boolean changesApplied = MergeEntriesHelper.mergeEntries(result.mergedEntry(), entry, compoundEdit);
+                          boolean changesApplied = MergeEntriesHelper.mergeEntries(result.mergedEntry(), entry, compoundEdit, keywordSeparator);
                           if (changesApplied) {
                               successfulUpdates++;
                               return entry.getCitationKey();

jabgui/src/main/java/org/jabref/gui/mergeentries/BatchEntryMergeWithFetchedDataAction.java

@@ -61,7 +61,8 @@ public void execute() {
                 entries,
                 fetcher,
                 undoManager,
-                notificationService);
+                notificationService,
+                preferences.getBibEntryPreferences().getKeywordSeparator());
 
         mergeTask.executeWith(taskExecutor);
     }

jabgui/src/main/java/org/jabref/gui/mergeentries/MergeEntriesHelper.java

@@ -10,8 +10,10 @@
 import org.jabref.logic.bibtex.comparator.ComparisonResult;
 import org.jabref.logic.bibtex.comparator.plausibility.PlausibilityComparatorFactory;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.KeywordList;
 import org.jabref.model.entry.field.Field;
 import org.jabref.model.entry.field.FieldFactory;
+import org.jabref.model.entry.field.StandardField;
 import org.jabref.model.entry.types.EntryType;
 
 import org.slf4j.Logger;
@@ -32,12 +34,13 @@ private MergeEntriesHelper() {
     /// @param entryFromFetcher  The entry containing new information (source, from the fetcher)
     /// @param entryFromLibrary  The entry to be updated (target, from the library)
     /// @param namedCompoundEdit Compound edit to collect undo information
-    public static boolean mergeEntries(BibEntry entryFromFetcher, BibEntry entryFromLibrary, NamedCompoundEdit namedCompoundEdit) {
+    /// @param keywordSeparator  Separator character used for union-merging the groups field
+    public static boolean mergeEntries(BibEntry entryFromFetcher, BibEntry entryFromLibrary, NamedCompoundEdit namedCompoundEdit, char keywordSeparator) {
         LOGGER.debug("Entry from fetcher: {}", entryFromFetcher);
         LOGGER.debug("Entry from library: {}", entryFromLibrary);
 
         boolean typeChanged = mergeEntryType(entryFromFetcher, entryFromLibrary, namedCompoundEdit);
-        boolean fieldsChanged = mergeFields(entryFromFetcher, entryFromLibrary, namedCompoundEdit);
+        boolean fieldsChanged = mergeFields(entryFromFetcher, entryFromLibrary, namedCompoundEdit, keywordSeparator);
         boolean fieldsRemoved = removeFieldsNotPresentInFetcher(entryFromFetcher, entryFromLibrary, namedCompoundEdit);
 
         return typeChanged || fieldsChanged || fieldsRemoved;
@@ -56,7 +59,7 @@ private static boolean mergeEntryType(BibEntry entryFromFetcher, BibEntry entryF
         return false;
     }
 
-    private static boolean mergeFields(BibEntry entryFromFetcher, BibEntry entryFromLibrary, NamedCompoundEdit namedCompoundEdit) {
+    private static boolean mergeFields(BibEntry entryFromFetcher, BibEntry entryFromLibrary, NamedCompoundEdit namedCompoundEdit, char keywordSeparator) {
         Set<Field> allFields = new LinkedHashSet<>();
         allFields.addAll(entryFromFetcher.getFields());
         allFields.addAll(entryFromLibrary.getFields());
@@ -67,7 +70,17 @@ private static boolean mergeFields(BibEntry entryFromFetcher, BibEntry entryFrom
             Optional<String> fetcherValue = entryFromFetcher.getField(field);
             Optional<String> libraryValue = entryFromLibrary.getField(field);
 
-            if (fetcherValue.isPresent() && shouldUpdateField(field, fetcherValue.get(), libraryValue)) {
+            if (field == StandardField.GROUPS && fetcherValue.isPresent()) {
+                // Always union-merge groups so no source group is ever lost
+                String merged = KeywordList.merge(libraryValue.orElse(""), fetcherValue.get(), keywordSeparator)
+                                           .getAsString(keywordSeparator);
+                if (!merged.equals(libraryValue.orElse(""))) {
+                    LOGGER.debug("Union-merging groups: {} + {} -> {}", libraryValue.orElse(""), fetcherValue.get(), merged);
+                    entryFromLibrary.setField(field, merged);
+                    namedCompoundEdit.addEdit(new UndoableFieldChange(entryFromLibrary, field, libraryValue.orElse(null), merged));
+                    anyFieldsChanged = true;
+                }
+            } else if (fetcherValue.isPresent() && shouldUpdateField(field, fetcherValue.get(), libraryValue)) {
                 LOGGER.debug("Updating field {}: {} -> {}", field, libraryValue.orElse(null), fetcherValue.get());
                 entryFromLibrary.setField(field, fetcherValue.get());
                 namedCompoundEdit.addEdit(new UndoableFieldChange(entryFromLibrary, field, libraryValue.orElse(null), fetcherValue.get()));
@@ -88,6 +101,10 @@ private static boolean removeFieldsNotPresentInFetcher(BibEntry entryFromFetcher
                 continue;
             }
 
+            if (field == StandardField.GROUPS) {
+                continue;
+            }
+
             Optional<String> value = entryFromLibrary.getField(field);
             if (value.isPresent()) {
                 LOGGER.debug("Removing obsolete field {} with value {}", field, value.get());

jabgui/src/test/java/org/jabref/gui/mergeentries/MergeEntriesHelperTest.java

@@ -0,0 +1,92 @@
+package org.jabref.gui.mergeentries;
+
+import org.jabref.gui.undo.NamedCompoundEdit;
+import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.field.StandardField;
+import org.jabref.model.entry.types.StandardEntryType;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+class MergeEntriesHelperTest {
+
+    private static final char KEYWORD_SEPARATOR = ',';
+
+    private NamedCompoundEdit compoundEdit;
+
+    @BeforeEach
+    void setup() {
+        compoundEdit = new NamedCompoundEdit("test");
+    }
+
+    @Test
+    void groupsFieldIsNotRemovedWhenFetcherHasNoGroups() {
+        BibEntry entryFromFetcher = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title");
+        BibEntry entryFromLibrary = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.GROUPS, "IEEE");
+
+        MergeEntriesHelper.mergeEntries(entryFromFetcher, entryFromLibrary, compoundEdit, KEYWORD_SEPARATOR);
+
+        assertEquals("IEEE", entryFromLibrary.getField(StandardField.GROUPS).orElse(""));
+    }
+
+    @Test
+    void groupsAreUnionMergedWhenBothEntriesHaveGroups() {
+        BibEntry entryFromFetcher = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.GROUPS, "Springer");
+        BibEntry entryFromLibrary = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.GROUPS, "IEEE");
+
+        MergeEntriesHelper.mergeEntries(entryFromFetcher, entryFromLibrary, compoundEdit, KEYWORD_SEPARATOR);
+
+        String groups = entryFromLibrary.getField(StandardField.GROUPS).orElse("");
+        assertEquals("IEEE, Springer", entryFromLibrary.getField(StandardField.GROUPS).orElse(""));
+    }
+
+    @Test
+    void groupsAreNotDuplicatedOnRepeatedMerge() {
+        BibEntry entryFromFetcher = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.GROUPS, "IEEE");
+        BibEntry entryFromLibrary = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.GROUPS, "IEEE");
+
+        MergeEntriesHelper.mergeEntries(entryFromFetcher, entryFromLibrary, compoundEdit, KEYWORD_SEPARATOR);
+
+        assertEquals("IEEE", entryFromLibrary.getField(StandardField.GROUPS).orElse(""));
+    }
+
+    @Test
+    void groupsFromFetcherAreAddedWhenLibraryHasNoGroups() {
+        BibEntry entryFromFetcher = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.GROUPS, "Springer");
+        BibEntry entryFromLibrary = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title");
+
+        MergeEntriesHelper.mergeEntries(entryFromFetcher, entryFromLibrary, compoundEdit, KEYWORD_SEPARATOR);
+
+        assertEquals("Springer", entryFromLibrary.getField(StandardField.GROUPS).orElse(""));
+    }
+
+    @Test
+    void regularObsoleteFieldIsRemovedWhenNotInFetcher() {
+        BibEntry entryFromFetcher = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title");
+        BibEntry entryFromLibrary = new BibEntry(StandardEntryType.Article)
+                .withField(StandardField.TITLE, "Some Title")
+                .withField(StandardField.NOTE, "Obsolete note");
+
+        MergeEntriesHelper.mergeEntries(entryFromFetcher, entryFromLibrary, compoundEdit, KEYWORD_SEPARATOR);
+
+        assertFalse(entryFromLibrary.hasField(StandardField.NOTE));
+    }
+}

jablib/src/main/java/org/jabref/logic/crawler/StudyRepository.java

@@ -7,7 +7,10 @@
 import java.nio.file.Path;
 import java.time.LocalDateTime;
 import java.time.temporal.ChronoUnit;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
@@ -28,7 +31,12 @@
 import org.jabref.logic.util.io.FileNameCleaner;
 import org.jabref.model.database.BibDatabase;
 import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.entry.BibEntry;
 import org.jabref.model.entry.BibEntryTypesManager;
+import org.jabref.model.groups.AllEntriesGroup;
+import org.jabref.model.groups.ExplicitGroup;
+import org.jabref.model.groups.GroupHierarchyType;
+import org.jabref.model.groups.GroupTreeNode;
 import org.jabref.model.metadata.SaveOrder;
 import org.jabref.model.metadata.SelfContainedSaveOrder;
 import org.jabref.model.study.FetchResult;
@@ -353,6 +361,7 @@ private String computeIDForQuery(String query) {
     private void persistResults(List<QueryResult> crawlResults) throws IOException, SaveException {
         DatabaseMerger merger = new DatabaseMerger(preferences.getBibEntryPreferences().getKeywordSeparator());
         BibDatabase newStudyResultEntries = new BibDatabase();
+        Map<String, List<BibEntry>> fetcherEntryMap = new LinkedHashMap<>();
 
         for (QueryResult result : crawlResults) {
             BibDatabase queryResultEntries = new BibDatabase();
@@ -366,6 +375,11 @@ private void persistResults(List<QueryResult> crawlResults) throws IOException,
                 // Create citation keys for all entries that do not have one
                 generateCiteKeys(existingFetcherResult, fetcherEntries);
 
+                // tag entries + add group to per-fetcher .bib
+                addFetcherGroup(existingFetcherResult, fetcherResult.getFetcherName(), fetcherEntries.getEntries());
+                fetcherEntryMap.computeIfAbsent(fetcherResult.getFetcherName(), k -> new ArrayList<>())
+                               .addAll(fetcherEntries.getEntries());
+
                 // Aggregate each fetcher result into the query result
                 merger.merge(queryResultEntries, fetcherEntries);
 
@@ -385,6 +399,10 @@ private void persistResults(List<QueryResult> crawlResults) throws IOException,
         // Merge new entries into study result file
         merger.merge(existingStudyResultEntries.getDatabase(), newStudyResultEntries);
 
+        // Add fetcher groups to final result.bib
+        fetcherEntryMap.forEach((fetcherName, entries) ->
+                addFetcherGroup(existingStudyResultEntries, fetcherName, entries));
+
         writeResultToFile(getPathToStudyResultFile(), existingStudyResultEntries);
     }
 
@@ -414,6 +432,42 @@ private void writeResultToFile(Path pathToFile, BibDatabaseContext context) thro
         }
     }
 
+    /// Creates an {@link ExplicitGroup} named after the fetcher and assigns all its entries to it.
+    /// If the group already exists in the database, new entries are added to it.
+    /// If no group tree exists yet in the context, a root {@link AllEntriesGroup} is created first.
+    ///
+    /// @param context     The database context to add the group to
+    /// @param fetcherName The name of the fetcher used as the group name
+    /// @param entries     The entries fetched from that fetcher to assign to the group
+    private void addFetcherGroup(BibDatabaseContext context, String fetcherName, List<BibEntry> entries) {
+        try {
+            ExplicitGroup group = new ExplicitGroup(
+                    fetcherName,
+                    GroupHierarchyType.INDEPENDENT,
+                    preferences.getBibEntryPreferences().getKeywordSeparator());
+
+            group.add(entries);
+
+            // Get existing root node or create a new AllEntriesGroup root if none exists yet
+            GroupTreeNode root = context.getMetaData().getGroups().orElseGet(() -> {
+                GroupTreeNode newRoot = GroupTreeNode.fromGroup(new AllEntriesGroup("All Entries"));
+                context.getMetaData().setGroups(newRoot);
+                return newRoot;
+            });
+
+            // add new entries to the existing group instead of creating a duplicate
+            root.getChildren().stream()
+                .filter(child -> fetcherName.equals(child.getGroup().getName()))
+                .findFirst()
+                .ifPresentOrElse(
+                        existingNode -> existingNode.addEntriesToGroup(entries),
+                        () -> root.addSubgroup(group)
+                );
+        } catch (IllegalArgumentException e) {
+            LOGGER.error("Problem adding fetcher group '{}' to database", fetcherName, e);
+        }
+    }
+
     private Path getPathToFetcherResultFile(String query, String fetcherName) {
         return repositoryPath.resolve(trimNameAndAddID(query)).resolve(FileNameCleaner.cleanFileName(fetcherName) + ".bib");
     }