Groups inline

CHANGELOG.md

@@ -14,6 +14,7 @@ to [sourceforge feature requests](https://sourceforge.net/p/jabref/features/) by
 ## [Unreleased]
 
 ### Changed
+- Implemented [#629](https://github.com/JabRef/jabref/issues/629): Explicit groups are now written in the "groups" field of the entry instead of at the end of the bib file
 - Main table now accepts pasted DOIs and tries to retrieve the entry
 - Added support for several Biblatex-fields through drop-down lists with valid alternatives
 - Added integrity checker for an odd number of unescaped '#'

src/main/java/net/sf/jabref/MetaData.java

@@ -38,7 +38,6 @@
 import net.sf.jabref.logic.labelpattern.DatabaseLabelPattern;
 import net.sf.jabref.logic.util.strings.StringUtil;
 import net.sf.jabref.migrations.VersionHandling;
-import net.sf.jabref.model.database.BibDatabase;
 import net.sf.jabref.model.database.BibDatabaseMode;
 import net.sf.jabref.sql.DBStrings;
 
@@ -78,7 +77,7 @@ public class MetaData implements Iterable<String> {
      * must simply make sure the appropriate changes are reflected in the Vector
      * it has been passed.
      */
-    public MetaData(Map<String, String> inData, BibDatabase db) {
+    public MetaData(Map<String, String> inData) {
         Objects.requireNonNull(inData);
         boolean groupsTreePresent = false;
         List<String> flatGroupsData = null;
@@ -117,7 +116,7 @@ public MetaData(Map<String, String> inData, BibDatabase db) {
 
         // this possibly handles import of a previous groups version
         if (groupsTreePresent) {
-            putGroups(treeGroupsData, db, groupsVersionOnDisk);
+            putGroups(treeGroupsData, groupsVersionOnDisk);
         }
 
         if (!groupsTreePresent && (flatGroupsData != null)) {
@@ -198,13 +197,11 @@ public void putData(String key, List<String> orderedData) {
      * Parse the groups metadata string
      *
      * @param orderedData The vector of metadata strings
-     * @param db          The BibDatabase this metadata belongs to
      * @param version     The group tree version
      */
-    private void putGroups(List<String> orderedData, BibDatabase db, int version) {
+    private void putGroups(List<String> orderedData, int version) {
         try {
-            groupsRoot = VersionHandling.importGroups(orderedData, db,
-                    version);
+            groupsRoot = VersionHandling.importGroups(orderedData, version);
             groupTreeValid = true;
         } catch (Exception e) {
             // we cannot really do anything about this here

src/main/java/net/sf/jabref/gui/groups/UndoableChangeAssignment.java

@@ -15,14 +15,13 @@
 */
 package net.sf.jabref.gui.groups;
 
-import java.util.HashSet;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 
 import javax.swing.undo.AbstractUndoableEdit;
 
-import net.sf.jabref.logic.groups.ExplicitGroup;
 import net.sf.jabref.logic.groups.GroupTreeNode;
 import net.sf.jabref.logic.l10n.Localization;
 import net.sf.jabref.model.entry.BibEntry;
@@ -32,8 +31,8 @@
  */
 public class UndoableChangeAssignment extends AbstractUndoableEdit {
 
-    private final Set<BibEntry> previousAssignments;
-    private final Set<BibEntry> newAssignments;
+    private final List<BibEntry> previousAssignments;
+    private final List<BibEntry> newAssignments;
     /**
      * The path to the edited node
      */
@@ -48,8 +47,8 @@ public class UndoableChangeAssignment extends AbstractUndoableEdit {
      */
     public UndoableChangeAssignment(GroupTreeNodeViewModel node, Set<BibEntry> previousAssignments,
             Set<BibEntry> newAssignments) {
-        this.previousAssignments = new HashSet<>(previousAssignments);
-        this.newAssignments = new HashSet<>(newAssignments);
+        this.previousAssignments = new ArrayList<>(previousAssignments);
+        this.newAssignments = new ArrayList<>(newAssignments);
         this.root = node.getNode().getRoot();
         this.pathToNode = node.getNode().getIndexedPathFromRoot();
     }
@@ -70,11 +69,7 @@ public void undo() {
 
         Optional<GroupTreeNode> node = root.getDescendant(pathToNode);
         if (node.isPresent()) {
-            ExplicitGroup group = (ExplicitGroup) node.get().getGroup();
-            group.clearAssignments();
-            for (final BibEntry entry : previousAssignments) {
-                group.addEntry(entry);
-            }
+            node.get().getGroup().add(previousAssignments);
         }
     }
 
@@ -84,11 +79,7 @@ public void redo() {
 
         Optional<GroupTreeNode> node = root.getDescendant(pathToNode);
         if (node.isPresent()) {
-            ExplicitGroup group = (ExplicitGroup) node.get().getGroup();
-            group.clearAssignments();
-            for (final BibEntry entry : newAssignments) {
-                group.addEntry(entry);
-            }
+            node.get().getGroup().add(newAssignments);
         }
     }
 }

src/main/java/net/sf/jabref/importer/AppendDatabaseAction.java

@@ -175,18 +175,13 @@ private static void mergeFromBibtex(JabRefFrame frame, BasePanel panel, ParserRe
                     // create a dummy group
                     ExplicitGroup group = new ExplicitGroup("Imported", GroupHierarchyType.INDEPENDENT);
                     newGroups.setGroup(group);
-                    appendedEntries.stream().map(group::addEntry);
+                    group.add(appendedEntries);
                 }
 
                 // groupsSelector is always created, even when no groups
                 // have been defined. therefore, no check for null is
                 // required here
                 frame.getGroupSelector().addGroups(newGroups, ce);
-
-                // for explicit groups, the entries copied to the mother fromDatabase have to
-                // be "reassigned", i.e. the old reference is removed and the reference
-                // to the new fromDatabase is added.
-                newGroups.replaceEntriesInExplicitGroup(originalEntries, appendedEntries);
             }
         }
 

src/main/java/net/sf/jabref/importer/ConvertLegacyExplicitGroups.java

@@ -0,0 +1,58 @@
+package net.sf.jabref.importer;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+import net.sf.jabref.gui.BasePanel;
+import net.sf.jabref.logic.groups.ExplicitGroup;
+import net.sf.jabref.logic.groups.GroupTreeNode;
+import net.sf.jabref.model.entry.BibEntry;
+
+/**
+ * Converts legacy explicit groups, where the group contained a list of assigned entries, to the new format,
+ * where the entry stores a list of groups it belongs to.
+ */
+public class ConvertLegacyExplicitGroups implements PostOpenAction {
+
+    @Override
+    public boolean isActionNecessary(ParserResult pr) {
+        if(pr.getMetaData().getGroups() == null) {
+            return false;
+        }
+        return !getExplicitGroupsWithLegacyKeys(pr.getMetaData().getGroups()).isEmpty();
+    }
+
+    @Override
+    public void performAction(BasePanel panel, ParserResult pr) {
+        Objects.requireNonNull(pr);
+        if(pr.getMetaData().getGroups() == null) {
+            return;
+        }
+
+        for (ExplicitGroup group : getExplicitGroupsWithLegacyKeys(pr.getMetaData().getGroups())) {
+            for (String entryKey : group.getLegacyEntryKeys()) {
+                for (BibEntry entry : pr.getDatabase().getEntriesByKey(entryKey)) {
+                    group.add(entry);
+                }
+            }
+            group.clearLegacyEntryKeys();
+        }
+    }
+
+    private List<ExplicitGroup> getExplicitGroupsWithLegacyKeys(GroupTreeNode node) {
+        Objects.requireNonNull(node);
+        List<ExplicitGroup> findings = new ArrayList<>();
+
+        if (node.getGroup() instanceof ExplicitGroup) {
+            ExplicitGroup group = (ExplicitGroup) node.getGroup();
+            if (!group.getLegacyEntryKeys().isEmpty()) {
+                findings.add(group);
+            }
+        }
+
+        node.getChildren().forEach(child -> findings.addAll(getExplicitGroupsWithLegacyKeys(child)));
+
+        return findings;
+    }
+}

src/main/java/net/sf/jabref/importer/OpenDatabaseAction.java

@@ -75,13 +75,14 @@ public class OpenDatabaseAction extends MnemonicAwareAction {
     private static final List<PostOpenAction> POST_OPEN_ACTIONS = new ArrayList<>();
 
     static {
-        // Add the action for checking for new custom entry types loaded from
-        // the bib file:
-        OpenDatabaseAction.POST_OPEN_ACTIONS.add(new CheckForNewEntryTypesAction());
+        // Add the action for checking for new custom entry types loaded from the bib file:
+        POST_OPEN_ACTIONS.add(new CheckForNewEntryTypesAction());
+        // Add the action for converting legacy entries in ExplicitGroup
+        POST_OPEN_ACTIONS.add(new ConvertLegacyExplicitGroups());
         // Add the action for the new external file handling system in version 2.3:
-        OpenDatabaseAction.POST_OPEN_ACTIONS.add(new FileLinksUpgradeWarning());
+        POST_OPEN_ACTIONS.add(new FileLinksUpgradeWarning());
         // Add the action for warning about and handling duplicate BibTeX keys:
-        OpenDatabaseAction.POST_OPEN_ACTIONS.add(new HandleDuplicateWarnings());
+        POST_OPEN_ACTIONS.add(new HandleDuplicateWarnings());
     }
 
     public OpenDatabaseAction(JabRefFrame frame, boolean showDialog) {

src/main/java/net/sf/jabref/importer/ParserResult.java

@@ -51,7 +51,7 @@ public class ParserResult {
 
 
     public ParserResult(Collection<BibEntry> entries) {
-        this(BibDatabases.createDatabase(BibDatabases.purgeEmptyEntries(entries)), null, new HashMap<>());
+        this(BibDatabases.createDatabase(BibDatabases.purgeEmptyEntries(entries)), new MetaData(), new HashMap<>());
     }
 
     public ParserResult(BibDatabase base, MetaData metaData, Map<String, EntryType> entryTypes) {

src/main/java/net/sf/jabref/importer/fileformat/BibtexParser.java

@@ -196,7 +196,7 @@ private ParserResult parseFileContent() throws IOException {
         }
 
         // Instantiate meta data:
-        parserResult.setMetaData(new MetaData(meta, database));
+        parserResult.setMetaData(new MetaData(meta));
 
         parseRemainingContent();
 

src/main/java/net/sf/jabref/logic/groups/AbstractGroup.java

@@ -15,6 +15,7 @@
 */
 package net.sf.jabref.logic.groups;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
@@ -60,7 +61,7 @@ public abstract class AbstractGroup implements SearchMatcher {
      * @throws Exception If an error occured and a group could not be created, e.g.
      *                   due to a malformed regular expression.
      */
-    public static AbstractGroup fromString(String s, BibDatabase db, int version) throws Exception {
+    public static AbstractGroup fromString(String s, int version) throws Exception {
         if (s.startsWith(KeywordGroup.ID)) {
             return KeywordGroup.fromString(s, version);
         }
@@ -71,7 +72,7 @@ public static AbstractGroup fromString(String s, BibDatabase db, int version) th
             return SearchGroup.fromString(s, version);
         }
         if (s.startsWith(ExplicitGroup.ID)) {
-            return ExplicitGroup.fromString(s, db, version);
+            return ExplicitGroup.fromString(s, version);
         }
         return null; // unknown group
     }
@@ -117,6 +118,10 @@ public final void setName(String name) {
      */
     public abstract Optional<EntriesGroupChange> add(List<BibEntry> entriesToAdd);
 
+    public Optional<EntriesGroupChange> add(BibEntry entryToAdd) {
+        return add(Collections.singletonList(entryToAdd));
+    }
+
     /**
      * Removes the specified entries from this group.
      *