Fix #3046: No longer allow duplicate fields in customized entry types (…

…#3405)

* Fix #3046: No longer allow duplicate fields in customized entry types

* Fix tests

* Fix tests 2

CHANGELOG.md

@@ -37,6 +37,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
  - We improved the way metadata is updated in remote databases. [#3235](https://github.com/JabRef/jabref/issues/3235)
  - We improved font rendering of the Entry Editor for Linux based systems [#3295](https://github.com/JabRef/jabref/issues/3295)
  - We fixed an issue where JabRef would freeze when trying to replace the original entry after a merge with new information from identifiers like DOI/ISBN etc. [3294](https://github.com/JabRef/jabref/issues/3294)
+ - We no longer allow to add a field multiple times in customized entry types and thereby fix an issue in the entry editor that resulted from having a field multiple times. [#3046](https://github.com/JabRef/jabref/issues/3046)
  - We fixed an issue where JabRef would not show the translated content at some points, although there existed a translation
  - We fixed an issue where editing in the source tab would override content of other entries [#3352](https://github.com/JabRef/jabref/issues/3352#issue-268580818)
  - We fixed several issues with the automatic linking of files in the entry editor where files were not found or not correctly saved in the bibtex source [#3346](https://github.com/JabRef/jabref/issues/3346)

src/main/java/org/jabref/gui/customentrytypes/EntryCustomizationDialog.java

@@ -66,9 +66,9 @@ public class EntryCustomizationDialog extends JabRefDialog implements ListSelect
     private JButton apply;
     private final List<String> preset = InternalBibtexFields.getAllPublicFieldNames();
     private String lastSelected;
-    private final Map<String, List<String>> reqLists = new HashMap<>();
-    private final Map<String, List<String>> optLists = new HashMap<>();
-    private final Map<String, List<String>> opt2Lists = new HashMap<>();
+    private final Map<String, Set<String>> reqLists = new HashMap<>();
+    private final Map<String, Set<String>> optLists = new HashMap<>();
+    private final Map<String, Set<String>> opt2Lists = new HashMap<>();
     private final Set<String> defaulted = new HashSet<>();
     private final Set<String> changed = new HashSet<>();
 
@@ -198,17 +198,17 @@ public void valueChanged(ListSelectionEvent e) {
         if (selectedTypeName == null) {
             return;
         }
-        List<String> requiredFieldsSelectedType = reqLists.get(selectedTypeName);
+        Set<String> requiredFieldsSelectedType = reqLists.get(selectedTypeName);
         if (requiredFieldsSelectedType == null) {
             Optional<EntryType> type = EntryTypes.getType(selectedTypeName, bibDatabaseMode);
             if (type.isPresent()) {
-                List<String> req = type.get().getRequiredFields();
+                Set<String> req = type.get().getRequiredFields();
 
-                List<String> opt;
+                Set<String> opt;
                 if (biblatexMode) {
                     opt = type.get().getPrimaryOptionalFields();
 
-                    List<String> opt2 = type.get().getSecondaryOptionalFields();
+                    Set<String> opt2 = type.get().getSecondaryOptionalFields();
 
                     optComp2.setFields(opt2);
                     optComp2.setEnabled(true);
@@ -221,12 +221,12 @@ public void valueChanged(ListSelectionEvent e) {
                 optComp.setEnabled(true);
             } else {
                 // New entry
-                reqComp.setFields(new ArrayList<>());
+                reqComp.setFields(new HashSet<>());
                 reqComp.setEnabled(true);
-                optComp.setFields(new ArrayList<>());
+                optComp.setFields(new HashSet<>());
                 optComp.setEnabled(true);
                 if (biblatexMode) {
-                    optComp2.setFields(new ArrayList<>());
+                    optComp2.setFields(new HashSet<>());
                     optComp2.setEnabled(true);
                 }
                 reqComp.requestFocus();
@@ -249,18 +249,18 @@ private void applyChanges() {
         List<String> actuallyChangedTypes = new ArrayList<>();
 
         // Iterate over our map of required fields, and list those types if necessary:
-        List<String> types = typeComp.getFields();
-        for (Map.Entry<String, List<String>> stringListEntry : reqLists.entrySet()) {
+        Set<String> types = typeComp.getFields();
+        for (Map.Entry<String, Set<String>> stringListEntry : reqLists.entrySet()) {
             if (!types.contains(stringListEntry.getKey())) {
                 continue;
             }
 
-            List<String> requiredFieldsList = stringListEntry.getValue();
-            List<String> optionalFieldsList = optLists.get(stringListEntry.getKey());
-            List<String> secondaryOptionalFieldsLists = opt2Lists.get(stringListEntry.getKey());
+            Set<String> requiredFieldsList = stringListEntry.getValue();
+            Set<String> optionalFieldsList = optLists.get(stringListEntry.getKey());
+            Set<String> secondaryOptionalFieldsLists = opt2Lists.get(stringListEntry.getKey());
 
             if (secondaryOptionalFieldsLists == null) {
-                secondaryOptionalFieldsLists = new ArrayList<>(0);
+                secondaryOptionalFieldsLists = new HashSet<>(0);
             }
 
             // If this type is already existing, check if any changes have
@@ -278,16 +278,16 @@ private void applyChanges() {
 
             Optional<EntryType> oldType = EntryTypes.getType(stringListEntry.getKey(), bibDatabaseMode);
             if (oldType.isPresent()) {
-                List<String> oldRequiredFieldsList = oldType.get().getRequiredFieldsFlat();
-                List<String> oldOptionalFieldsList = oldType.get().getOptionalFields();
+                Set<String> oldRequiredFieldsList = oldType.get().getRequiredFieldsFlat();
+                Set<String> oldOptionalFieldsList = oldType.get().getOptionalFields();
                 if (biblatexMode) {
-                    List<String> oldPrimaryOptionalFieldsLists = oldType.get().getPrimaryOptionalFields();
-                    List<String> oldSecondaryOptionalFieldsList = oldType.get().getSecondaryOptionalFields();
-                    if (equalLists(oldRequiredFieldsList, requiredFieldsList) && equalLists(oldPrimaryOptionalFieldsLists, optionalFieldsList) &&
-                            equalLists(oldSecondaryOptionalFieldsList, secondaryOptionalFieldsLists)) {
+                    Set<String> oldPrimaryOptionalFieldsLists = oldType.get().getPrimaryOptionalFields();
+                    Set<String> oldSecondaryOptionalFieldsList = oldType.get().getSecondaryOptionalFields();
+                    if (oldRequiredFieldsList.equals(requiredFieldsList) && oldPrimaryOptionalFieldsLists.equals(optionalFieldsList) &&
+                            oldSecondaryOptionalFieldsList.equals(secondaryOptionalFieldsLists)) {
                         changesMade = false;
                     }
-                } else if (equalLists(oldRequiredFieldsList, requiredFieldsList) && equalLists(oldOptionalFieldsList, optionalFieldsList)) {
+                } else if (oldRequiredFieldsList.equals(requiredFieldsList) && oldOptionalFieldsList.equals(optionalFieldsList)) {
                     changesMade = false;
                 }
             }
@@ -352,26 +352,6 @@ private void deleteType(String name) {
         }
     }
 
-    private static boolean equalLists(List<String> one, List<String> two) {
-        if ((one == null) && (two == null)) {
-            return true; // Both null.
-        }
-        if ((one == null) || (two == null)) {
-            return false; // One of them null, the other not.
-        }
-        if (one.size() != two.size()) {
-            return false; // Different length.
-        }
-        // If we get here, we know that both are non-null, and that they have the same length.
-        for (int i = 0; i < one.size(); i++) {
-            if (!one.get(i).equals(two.get(i))) {
-                return false;
-            }
-        }
-        // If we get here, all entries have matched.
-        return true;
-    }
-
     private void updateEntriesForChangedTypes(List<String> actuallyChangedTypes) {
         for (BasePanel bp : frame.getBasePanelList()) {
             // get all affected entries
@@ -403,10 +383,10 @@ public void actionPerformed(ActionEvent e) {
 
             Optional<EntryType> type = EntryTypes.getStandardType(lastSelected, bibDatabaseMode);
             if (type.isPresent()) {
-                List<String> of = type.get().getOptionalFields();
-                List<String> req = type.get().getRequiredFields();
-                List<String> opt1 = new ArrayList<>();
-                List<String> opt2 = new ArrayList<>();
+                Set<String> of = type.get().getOptionalFields();
+                Set<String> req = type.get().getRequiredFields();
+                Set<String> opt1 = new HashSet<>();
+                Set<String> opt2 = new HashSet<>();
 
                 if (!(of.isEmpty())) {
                     if (biblatexMode) {

src/main/java/org/jabref/gui/customentrytypes/FieldSetComponent.java

@@ -9,7 +9,6 @@
 import java.awt.event.ActionListener;
 import java.awt.event.FocusEvent;
 import java.awt.event.FocusListener;
-import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.HashSet;
 import java.util.List;
@@ -217,16 +216,16 @@ public void setEnabled(boolean en) {
         remove.setEnabled(en);
     }
 
-    public void setFields(List<String> fields) {
-        DefaultListModel<String> newListModel = new DefaultListModel<>();
-        for (String field : fields) {
-            newListModel.addElement(field);
-        }
-        this.listModel = newListModel;
-        for (ListDataListener modelListener : modelListeners) {
-            newListModel.addListDataListener(modelListener);
+    /**
+     * Return the current list.
+     */
+    public Set<String> getFields() {
+        Set<String> res = new HashSet<>(listModel.getSize());
+        Enumeration<String> elements = listModel.elements();
+        while (elements.hasMoreElements()) {
+            res.add(elements.nextElement());
         }
-        list.setModel(newListModel);
+        return res;
     }
 
     /**
@@ -279,16 +278,16 @@ protected void removeSelected() {
 
     }
 
-    /**
-     * Return the current list.
-     */
-    public List<String> getFields() {
-        List<String> res = new ArrayList<>(listModel.getSize());
-        Enumeration<String> elements = listModel.elements();
-        while (elements.hasMoreElements()) {
-            res.add(elements.nextElement());
+    public void setFields(Set<String> fields) {
+        DefaultListModel<String> newListModel = new DefaultListModel<>();
+        for (String field : fields) {
+            newListModel.addElement(field);
         }
-        return res;
+        this.listModel = newListModel;
+        for (ListDataListener modelListener : modelListeners) {
+            newListModel.addListDataListener(modelListener);
+        }
+        list.setModel(newListModel);
     }
 
     /**

src/main/java/org/jabref/gui/entryeditor/DeprecatedFieldsTab.java

@@ -1,6 +1,6 @@
 package org.jabref.gui.entryeditor;
 
-import java.util.List;
+import java.util.Collection;
 
 import javafx.scene.control.Tooltip;
 
@@ -21,7 +21,7 @@ public DeprecatedFieldsTab(BibDatabaseContext databaseContext, SuggestionProvide
     }
 
     @Override
-    protected List<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
+    protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
         return entryType.getDeprecatedFields();
     }
 }

src/main/java/org/jabref/gui/entryeditor/FieldsEditorTab.java

@@ -1,6 +1,7 @@
 package org.jabref.gui.entryeditor;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -69,7 +70,7 @@ private Region setupPanel(BibEntry entry, boolean compressed, SuggestionProvider
         List<Label> labels = new ArrayList<>();
 
         EntryType entryType = EntryTypes.getTypeOrDefault(entry.getType(), databaseContext.getMode());
-        List<String> fields = determineFieldsToShow(entry, entryType);
+        Collection<String> fields = determineFieldsToShow(entry, entryType);
         for (String fieldName : fields) {
             FieldEditorFX fieldEditor = FieldEditors.getForField(fieldName, Globals.TASK_EXECUTOR, new FXDialogService(),
                     Globals.journalAbbreviationLoader, Globals.prefs.getJournalAbbreviationPreferences(), Globals.prefs,
@@ -138,7 +139,7 @@ private Region setupPanel(BibEntry entry, boolean compressed, SuggestionProvider
         return scrollPane;
     }
 
-    private void setRegularRowLayout(GridPane gridPane, List<String> fields, int rows) {
+    private void setRegularRowLayout(GridPane gridPane, Collection<String> fields, int rows) {
         List<RowConstraints> constraints = new ArrayList<>(rows);
         for (String field : fields) {
             RowConstraints rowExpand = new RowConstraints();
@@ -220,5 +221,5 @@ protected void bindToEntry(BibEntry entry) {
         setContent(panel);
     }
 
-    protected abstract List<String> determineFieldsToShow(BibEntry entry, EntryType entryType);
+    protected abstract Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType);
 }

src/main/java/org/jabref/gui/entryeditor/OptionalFields2Tab.java

@@ -1,6 +1,6 @@
 package org.jabref.gui.entryeditor;
 
-import java.util.List;
+import java.util.Collection;
 
 import javafx.scene.control.Tooltip;
 
@@ -21,7 +21,7 @@ public OptionalFields2Tab(BibDatabaseContext databaseContext, SuggestionProvider
     }
 
     @Override
-    protected List<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
+    protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
         return entryType.getSecondaryOptionalNotDeprecatedFields();
     }
 }

src/main/java/org/jabref/gui/entryeditor/OptionalFieldsTab.java

@@ -1,6 +1,6 @@
 package org.jabref.gui.entryeditor;
 
-import java.util.List;
+import java.util.Collection;
 
 import javafx.scene.control.Tooltip;
 
@@ -21,7 +21,7 @@ public OptionalFieldsTab(BibDatabaseContext databaseContext, SuggestionProviders
     }
 
     @Override
-    protected List<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
+    protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
         return entryType.getPrimaryOptionalFields();
     }
 }

src/main/java/org/jabref/gui/entryeditor/OtherFieldsTab.java

@@ -1,5 +1,6 @@
 package org.jabref.gui.entryeditor;
 
+import java.util.Collection;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -37,7 +38,7 @@ public static boolean isOtherField(EntryType entryType, String fieldToCheck) {
     }
 
     @Override
-    protected List<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
+    protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
         List<String> allKnownFields = entryType.getAllFields().stream().map(String::toLowerCase)
                 .collect(Collectors.toList());
         List<String> otherFields = entry.getFieldNames().stream().map(String::toLowerCase)

src/main/java/org/jabref/gui/entryeditor/RequiredFieldsTab.java

@@ -1,7 +1,8 @@
 package org.jabref.gui.entryeditor;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.Set;
 
 import javafx.scene.control.Tooltip;
 
@@ -22,8 +23,8 @@ public RequiredFieldsTab(BibDatabaseContext databaseContext, SuggestionProviders
     }
 
     @Override
-    protected List<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
-        List<String> fields = new ArrayList<>();
+    protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
+        Set<String> fields = new LinkedHashSet<>();
         fields.addAll(entryType.getRequiredFieldsFlat());
 
         // Add the edit field for Bibtex-key.

src/main/java/org/jabref/gui/entryeditor/UserDefinedFieldsTab.java

@@ -1,5 +1,6 @@
 package org.jabref.gui.entryeditor;
 
+import java.util.Collection;
 import java.util.List;
 
 import org.jabref.gui.IconTheme;
@@ -20,7 +21,7 @@ public UserDefinedFieldsTab(String name, List<String> fields, BibDatabaseContext
     }
 
     @Override
-    protected List<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
+    protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
         return fields;
     }
 }

src/main/java/org/jabref/logic/bibtex/BibEntryWriter.java

@@ -2,8 +2,8 @@
 
 import java.io.IOException;
 import java.io.Writer;
+import java.util.Collection;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Locale;
 import java.util.Optional;
 import java.util.Set;
@@ -95,7 +95,7 @@ private void writeRequiredFieldsFirstRemainingFieldsSecond(BibEntry entry, Write
         EntryType type = EntryTypes.getTypeOrDefault(entry.getType(), bibDatabaseMode);
 
         // Write required fields first.
-        List<String> fields = type.getRequiredFieldsFlat();
+        Collection<String> fields = type.getRequiredFieldsFlat();
         if (fields != null) {
             for (String value : fields) {
                 writeField(entry, out, value, indentation);