Preserve casing of field names for customize entry types (#9993)

* Preserve casing of field names for customize entry types * Update CHANGELOG.md * Fix checkstyle * Add BibEntryTypePrefsAndFileViewModel * Add BibEntry #withChanged * Replace faux-pas by jool (seems to be more maintained and more easy to use) * Use List.of() * Insist on SortedSet * Add comment on final list * Fix test * Keep casing of known fields * Fix tests
JabRef · Jun 12, 2023 · cd2d816 · cd2d816
1 parent 354c55f
commit cd2d816
Show file tree

Hide file tree

Showing 32 changed files with 332 additions and 143 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -53,6 +53,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We improved the Medline importer to correctly import ISO dates for `revised`. [#9536](https://github.com/JabRef/jabref/issues/9536)
 - To avoid cluttering of the directory, We always delete the `.sav` file upon successful write. [#9675](https://github.com/JabRef/jabref/pull/9675)
 - We improved the unlinking/deletion of multiple linked files of an entry using the <kbd>Delete</kbd> key. [#9473](https://github.com/JabRef/jabref/issues/9473)
+- The field names of customized entry types are now exchanged preserving the case. [#9993](https://github.com/JabRef/jabref/pull/9993)
 - We moved the custom entry types dialog into the preferences dialog. [#9760](https://github.com/JabRef/jabref/pull/9760)
 - We moved the manage content selectors dialog to the library properties. [#9768](https://github.com/JabRef/jabref/pull/9768)
 - We moved the preferences menu command from the options menu to the file menu. [#9768](https://github.com/JabRef/jabref/pull/9768)

diff --git a/build.gradle b/build.gradle
@@ -201,7 +201,8 @@ dependencies {
     implementation 'com.vladsch.flexmark:flexmark:0.64.8'
 
     implementation group: 'net.harawata', name: 'appdirs', version: '1.2.1'
-    implementation group: 'org.zalando', name: 'faux-pas', version: '0.9.0'
+
+    implementation group: 'org.jooq', name: 'jool', version: '0.9.15'
 
     testImplementation 'io.github.classgraph:classgraph:4.8.160'
     testImplementation 'org.junit.jupiter:junit-jupiter:5.9.3'

diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java
@@ -96,9 +96,10 @@
     requires flexmark;
     requires flexmark.util.ast;
     requires flexmark.util.data;
+
     requires com.h2database.mvstore;
 
-    requires faux.pas;
+    requires org.jooq.jool;
 
     // fulltext search
     requires org.apache.lucene.core;

diff --git a/src/main/java/org/jabref/gui/fieldeditors/EditorTextArea.java b/src/main/java/org/jabref/gui/fieldeditors/EditorTextArea.java
@@ -76,7 +76,6 @@ public void paste() {
      */
     @FunctionalInterface
     public interface PasteActionHandler {
-
         void handle();
     }
 }
diff --git a/src/main/java/org/jabref/gui/importer/BibEntryTypePrefsAndFileViewModel.java b/src/main/java/org/jabref/gui/importer/BibEntryTypePrefsAndFileViewModel.java
@@ -0,0 +1,18 @@
+package org.jabref.gui.importer;
+
+import org.jabref.logic.exporter.MetaDataSerializer;
+import org.jabref.logic.l10n.Localization;
+import org.jabref.model.entry.BibEntryType;
+
+public record BibEntryTypePrefsAndFileViewModel(BibEntryType customTypeFromPreferences, BibEntryType customTypeFromFile) {
+    /**
+     * Used to render in the UI. This is different from {@link BibEntryType#toString()}, because this is the serialization the user expects
+     */
+    @Override
+    public String toString() {
+        return Localization.lang("%0 (from file)\n%1 (current setting)",
+                MetaDataSerializer.serializeCustomEntryTypes(customTypeFromFile),
+                MetaDataSerializer.serializeCustomEntryTypes(customTypeFromPreferences));
+    }
+}
+
diff --git a/src/main/java/org/jabref/gui/importer/ImportCustomEntryTypesDialog.java b/src/main/java/org/jabref/gui/importer/ImportCustomEntryTypesDialog.java
@@ -21,10 +21,12 @@
 public class ImportCustomEntryTypesDialog extends BaseDialog<Void> {
 
     private final List<BibEntryType> customEntryTypes;
+
+    @Inject private PreferencesService preferencesService;
     @FXML private VBox boxDifferentCustomization;
+
     @FXML private CheckListView<BibEntryType> unknownEntryTypesCheckList;
-    @Inject private PreferencesService preferencesService;
-    @FXML private CheckListView<BibEntryType> differentCustomizationCheckList;
+    @FXML private CheckListView<BibEntryTypePrefsAndFileViewModel> differentCustomizationCheckList;
 
     private final BibDatabaseMode mode;
     private ImportCustomEntryTypesDialogViewModel viewModel;
@@ -39,7 +41,11 @@ public ImportCustomEntryTypesDialog(BibDatabaseMode mode, List<BibEntryType> cus
 
         setResultConverter(btn -> {
             if (btn == ButtonType.OK) {
-                viewModel.importBibEntryTypes(unknownEntryTypesCheckList.getCheckModel().getCheckedItems(), differentCustomizationCheckList.getCheckModel().getCheckedItems());
+                viewModel.importBibEntryTypes(
+                        unknownEntryTypesCheckList.getCheckModel().getCheckedItems(),
+                        differentCustomizationCheckList.getCheckModel().getCheckedItems().stream()
+                                .map(BibEntryTypePrefsAndFileViewModel::customTypeFromPreferences)
+                                .toList());
             }
             return null;
         });

diff --git a/src/main/java/org/jabref/gui/importer/ImportCustomEntryTypesDialogViewModel.java b/src/main/java/org/jabref/gui/importer/ImportCustomEntryTypesDialogViewModel.java
@@ -12,13 +12,18 @@
 import org.jabref.model.entry.types.EntryTypeFactory;
 import org.jabref.preferences.PreferencesService;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class ImportCustomEntryTypesDialogViewModel {
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(ImportCustomEntryTypesDialogViewModel.class);
+
     private final BibDatabaseMode mode;
     private final PreferencesService preferencesService;
 
     private final ObservableList<BibEntryType> newTypes = FXCollections.observableArrayList();
-    private final ObservableList<BibEntryType> differentCustomizationTypes = FXCollections.observableArrayList();
+    private final ObservableList<BibEntryTypePrefsAndFileViewModel> differentCustomizationTypes = FXCollections.observableArrayList();
 
     public ImportCustomEntryTypesDialogViewModel(BibDatabaseMode mode, List<BibEntryType> entryTypes, PreferencesService preferencesService) {
         this.mode = mode;
@@ -29,8 +34,10 @@ public ImportCustomEntryTypesDialogViewModel(BibDatabaseMode mode, List<BibEntry
             if (currentlyStoredType.isEmpty()) {
                 newTypes.add(customType);
             } else {
-                if (!EntryTypeFactory.isEqualNameAndFieldBased(customType, currentlyStoredType.get())) {
-                    differentCustomizationTypes.add(customType);
+                if (!EntryTypeFactory.nameAndFieldsAreEqual(customType, currentlyStoredType.get())) {
+                    LOGGER.info("currently stored type:    {}", currentlyStoredType.get());
+                    LOGGER.info("type provided by library: {}", customType);
+                    differentCustomizationTypes.add(new BibEntryTypePrefsAndFileViewModel(currentlyStoredType.get(), customType));
                 }
             }
         }
@@ -40,7 +47,7 @@ public ObservableList<BibEntryType> newTypes() {
         return this.newTypes;
     }
 
-    public ObservableList<BibEntryType> differentCustomizations() {
+    public ObservableList<BibEntryTypePrefsAndFileViewModel> differentCustomizations() {
         return this.differentCustomizationTypes;
     }
 

diff --git a/src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesView.java b/src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesView.java
@@ -72,7 +72,6 @@ public void initialize() {
                 dialogService.showErrorDialogAndWait(Localization.lang(
                         "A string with the label '%0' already exists.",
                         cellEvent.getNewValue()));
-
                 cellItem.setLabel(cellEvent.getOldValue());
             } else {
                 cellItem.setLabel(cellEvent.getNewValue());

diff --git a/src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java b/src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTab.java
@@ -35,8 +35,6 @@
 import org.jabref.model.database.BibDatabaseMode;
 import org.jabref.model.entry.BibEntryTypesManager;
 import org.jabref.model.entry.field.Field;
-import org.jabref.model.entry.field.UnknownField;
-import org.jabref.model.strings.StringUtil;
 
 import com.airhacks.afterburner.views.ViewLoader;
 import com.tobiasdiez.easybind.EasyBind;
@@ -146,30 +144,32 @@ private void setupEntryTypesTable() {
     }
 
     private void setupFieldsTable() {
-        fieldNameColumn.setCellValueFactory(item -> item.getValue().nameProperty());
+        fieldNameColumn.setCellValueFactory(item -> item.getValue().displayNameProperty());
         fieldNameColumn.setCellFactory(TextFieldTableCell.forTableColumn());
         fieldNameColumn.setEditable(true);
-        fieldNameColumn.setOnEditCommit(
-                (TableColumn.CellEditEvent<FieldViewModel, String> event) -> {
-                    // This makes the displayed name consistent to org.jabref.model.entry.field.Field #getDisplayName()
-                    String newFieldValue = StringUtil.capitalizeFirst(event.getNewValue());
-                    UnknownField field = (UnknownField) event.getRowValue().getField();
-                    EntryTypeViewModel selectedEntryType = viewModel.selectedEntryTypeProperty().get();
-                    ObservableList<FieldViewModel> entryFields = selectedEntryType.fields();
-                    // The first predicate will check if the user input the original field name or doesn't edit anything after double click
-                    boolean fieldExists = !newFieldValue.equals(field.getDisplayName()) && entryFields.stream().anyMatch(fieldViewModel ->
-                            fieldViewModel.nameProperty().getValue().equalsIgnoreCase(newFieldValue));
-
-                    if (fieldExists) {
-                        dialogService.notify(Localization.lang("Unable to change field name. \"%0\" already in use.", newFieldValue));
-                        event.getTableView().edit(-1, null);
-                        event.getTableView().refresh();
-                    } else {
-                        event.getRowValue().setField(newFieldValue);
-                        field.setName(newFieldValue);
-                        event.getTableView().refresh();
-                    }
-                });
+        fieldNameColumn.setOnEditCommit((TableColumn.CellEditEvent<FieldViewModel, String> event) -> {
+            String newDisplayName = event.getNewValue();
+            if (newDisplayName.isBlank()) {
+                dialogService.notify(Localization.lang("Name cannot be empty"));
+                event.getTableView().edit(-1, null);
+                event.getTableView().refresh();
+                return;
+            }
+
+            FieldViewModel fieldViewModel = event.getRowValue();
+            String currentDisplayName = fieldViewModel.displayNameProperty().getValue();
+            EntryTypeViewModel selectedEntryType = viewModel.selectedEntryTypeProperty().get();
+            ObservableList<FieldViewModel> entryFields = selectedEntryType.fields();
+            // The first predicate will check if the user input the original field name or doesn't edit anything after double click
+            boolean fieldExists = !newDisplayName.equals(currentDisplayName) && viewModel.displayNameExists(newDisplayName);
+            if (fieldExists) {
+                dialogService.notify(Localization.lang("Unable to change field name. \"%0\" already in use.", newDisplayName));
+                event.getTableView().edit(-1, null);
+            } else {
+                fieldViewModel.displayNameProperty().setValue(newDisplayName);
+            }
+            event.getTableView().refresh();
+        });
 
         fieldTypeColumn.setCellFactory(CheckBoxTableCell.forTableColumn(fieldTypeColumn));
         fieldTypeColumn.setCellValueFactory(item -> item.getValue().requiredProperty());
@@ -182,7 +182,7 @@ private void setupFieldsTable() {
         fieldTypeActionColumn.setSortable(false);
         fieldTypeActionColumn.setReorderable(false);
         fieldTypeActionColumn.setEditable(false);
-        fieldTypeActionColumn.setCellValueFactory(cellData -> cellData.getValue().nameProperty());
+        fieldTypeActionColumn.setCellValueFactory(cellData -> cellData.getValue().displayNameProperty());
 
         new ValueTableCellFactory<FieldViewModel, String>()
                 .withGraphic(item -> IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())

diff --git a/src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTabViewModel.java b/src/main/java/org/jabref/gui/preferences/customentrytypes/CustomEntryTypesTabViewModel.java
@@ -81,7 +81,7 @@ public CustomEntryTypesTabViewModel(BibDatabaseMode mode,
     }
 
     public void setValues() {
-        if (this.entryTypesWithFields.size() > 0) {
+        if (!this.entryTypesWithFields.isEmpty()) {
             this.entryTypesWithFields.clear();
         }
         Collection<BibEntryType> allTypes = entryTypesManager.getAllTypes(bibDatabaseMode);
@@ -105,12 +105,12 @@ public void storeSettings() {
 
             multilineFields.addAll(allFields.stream()
                                             .filter(FieldViewModel::isMultiline)
-                                            .map(FieldViewModel::getField)
+                                            .map(FieldViewModel::toField)
                                             .toList());
 
             List<OrFields> required = allFields.stream()
                                                .filter(FieldViewModel::isRequired)
-                                               .map(FieldViewModel::getField)
+                                               .map(FieldViewModel::toField)
                                                .map(OrFields::new)
                                                .collect(Collectors.toList());
             List<BibField> fields = allFields.stream().map(FieldViewModel::toBibField).collect(Collectors.toList());
@@ -144,24 +144,28 @@ public void removeEntryType(EntryTypeViewModel focusedItem) {
 
     public void addNewField() {
         Field field = newFieldToAdd.getValue();
-        ObservableList<FieldViewModel> entryFields = this.selectedEntryType.getValue().fields();
-        boolean fieldExists = entryFields.stream().anyMatch(fieldViewModel ->
-                fieldViewModel.nameProperty().getValue().equals(field.getDisplayName()));
+        boolean fieldExists = displayNameExists(field.getDisplayName());
 
-        if (!fieldExists) {
+        if (fieldExists) {
+            dialogService.showWarningDialogAndWait(
+                    Localization.lang("Duplicate fields"),
+                    Localization.lang("Warning: You added field \"%0\" twice. Only one will be kept.", field.getDisplayName()));
+        } else {
             this.selectedEntryType.getValue().addField(new FieldViewModel(
                     field,
                     FieldViewModel.Mandatory.REQUIRED,
                     FieldPriority.IMPORTANT,
                     false));
-        } else {
-            dialogService.showWarningDialogAndWait(
-                    Localization.lang("Duplicate fields"),
-                    Localization.lang("Warning: You added field \"%0\" twice. Only one will be kept.", field.getDisplayName()));
         }
         newFieldToAddProperty().setValue(null);
     }
 
+    public boolean displayNameExists(String displayName) {
+        ObservableList<FieldViewModel> entryFields = this.selectedEntryType.getValue().fields();
+        return entryFields.stream().anyMatch(fieldViewModel ->
+                fieldViewModel.displayNameProperty().getValue().equals(displayName));
+    }
+
     public void removeField(FieldViewModel focusedItem) {
         selectedEntryType.getValue().removeField(focusedItem);
     }

diff --git a/src/main/java/org/jabref/gui/preferences/customentrytypes/EntryTypeViewModel.java b/src/main/java/org/jabref/gui/preferences/customentrytypes/EntryTypeViewModel.java
@@ -22,7 +22,6 @@ public class EntryTypeViewModel {
 
     public EntryTypeViewModel(BibEntryType entryType, Predicate<Field> isMultiline) {
         this.entryType.set(entryType);
-
         List<FieldViewModel> allFieldsForType = entryType.getAllBibFields()
                        .stream().map(bibField -> new FieldViewModel(bibField.field(),
                                    entryType.isRequired(bibField.field()) ? Mandatory.REQUIRED : Mandatory.OPTIONAL,

diff --git a/src/main/java/org/jabref/gui/preferences/customentrytypes/FieldViewModel.java b/src/main/java/org/jabref/gui/preferences/customentrytypes/FieldViewModel.java
@@ -10,15 +10,13 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.entry.field.BibField;
 import org.jabref.model.entry.field.Field;
+import org.jabref.model.entry.field.FieldFactory;
 import org.jabref.model.entry.field.FieldPriority;
 import org.jabref.model.entry.field.FieldProperty;
 
-import com.tobiasdiez.easybind.EasyBind;
-
 public class FieldViewModel {
 
-    private final Field field;
-    private final StringProperty fieldName = new SimpleStringProperty("");
+    private final StringProperty displayName = new SimpleStringProperty("");
     private final BooleanProperty required = new SimpleBooleanProperty();
     private final BooleanProperty multiline = new SimpleBooleanProperty();
     private final ObjectProperty<FieldPriority> priorityProperty = new SimpleObjectProperty<>();
@@ -27,27 +25,14 @@ public FieldViewModel(Field field,
                           Mandatory required,
                           FieldPriority priorityProperty,
                           boolean multiline) {
-        this.field = field;
-        this.fieldName.setValue(field.getDisplayName());
+        this.displayName.setValue(field.getDisplayName());
         this.required.setValue(required == Mandatory.REQUIRED);
         this.priorityProperty.setValue(priorityProperty);
         this.multiline.setValue(multiline);
-
-        EasyBind.subscribe(this.multiline, multi -> {
-            if (multi) {
-                this.field.getProperties().add(FieldProperty.MULTILINE_TEXT);
-            } else {
-                this.field.getProperties().remove(FieldProperty.MULTILINE_TEXT);
-            }
-        });
-    }
-
-    public Field getField() {
-        return field;
     }
 
-    public StringProperty nameProperty() {
-        return fieldName;
+    public StringProperty displayNameProperty() {
+        return displayName;
     }
 
     public BooleanProperty requiredProperty() {
@@ -70,21 +55,26 @@ public FieldPriority getPriority() {
         return priorityProperty.getValue();
     }
 
-    public void setField(String field) {
-        this.fieldName.setValue(field);
+    public Field toField() {
+        // If the field name is known by JabRef, JabRef's casing will win.
+        // If the field is not known by JabRef (UnknownField), the new casing will be taken.
+        Field field = FieldFactory.parseField(displayName.getValue());
+        if (multiline.getValue()) {
+            field.getProperties().add(FieldProperty.MULTILINE_TEXT);
+        }
+        return field;
     }
 
     public BibField toBibField() {
-        return new BibField(field, priorityProperty.getValue());
+        return new BibField(toField(), priorityProperty.getValue());
     }
 
     @Override
     public String toString() {
-        return field.getDisplayName();
+        return displayName.getValue();
     }
 
     public enum Mandatory {
-
         REQUIRED(Localization.lang("Required")),
         OPTIONAL(Localization.lang("Optional"));