Merge remote-tracking branch 'upstream/master' into converteditextern…

…alfiles

* upstream/master:
  Fix saving of Strings (#4876)

src/main/java/org/jabref/gui/metadata/BibtexStringEditorDialogView.java

@@ -1,5 +1,7 @@
 package org.jabref.gui.metadata;
 
+import javax.inject.Inject;
+
 import javafx.event.ActionEvent;
 import javafx.fxml.FXML;
 import javafx.scene.control.Button;
@@ -10,6 +12,7 @@
 import javafx.scene.control.Tooltip;
 import javafx.util.converter.DefaultStringConverter;
 
+import org.jabref.gui.DialogService;
 import org.jabref.gui.icon.IconTheme.JabRefIcons;
 import org.jabref.gui.util.BaseDialog;
 import org.jabref.gui.util.IconValidationDecorator;
@@ -34,6 +37,8 @@ public class BibtexStringEditorDialogView extends BaseDialog<Void> {
     private final ControlsFxVisualizer visualizer = new ControlsFxVisualizer();
     private final BibtexStringEditorDialogViewModel viewModel;
 
+    @Inject private DialogService dialogService;
+
     public BibtexStringEditorDialogView(BibDatabase database) {
         viewModel = new BibtexStringEditorDialogViewModel(database);
 
@@ -72,7 +77,15 @@ private void initialize() {
         new ViewModelTextFieldTableCellVisualizationFactory<BibtexStringViewModel, String>().withValidation(BibtexStringViewModel::contentValidation, visualizer).install(colContent, new DefaultStringConverter());
 
         colLabel.setOnEditCommit((CellEditEvent<BibtexStringViewModel, String> cell) -> {
-            cell.getRowValue().setLabel(cell.getNewValue());
+
+            String newLabelValue = cell.getNewValue();
+            if (cell.getTableView().getItems().stream().anyMatch(strs -> strs.labelProperty().get().equals(newLabelValue))) {
+
+                dialogService.showErrorDialogAndWait(Localization.lang("A string with the label '%0' already exists.", newLabelValue));
+                cell.getRowValue().setLabel("");
+            } else {
+                cell.getRowValue().setLabel(cell.getNewValue());
+            }
         });
         colContent.setOnEditCommit((CellEditEvent<BibtexStringViewModel, String> cell) -> {
             cell.getRowValue().setContent(cell.getNewValue());

src/main/java/org/jabref/gui/metadata/BibtexStringEditorDialogViewModel.java

@@ -35,9 +35,7 @@ public BibtexStringEditorDialogViewModel(BibDatabase bibDatabase) {
         addAllStringsFromDB();
 
         ObservableList<ObservableValue<Boolean>> allValidProperty = EasyBind.map(allStringsProperty(), BibtexStringViewModel::combinedValidationValidProperty);
-
         validProperty.bind(EasyBind.combine(allValidProperty, stream -> stream.allMatch(valid -> valid)));
-
     }
 
     private void addAllStringsFromDB() {
@@ -59,7 +57,8 @@ public void addNewString() {
     }
 
     public void removeString() {
-        allStrings.remove(selectedItemProperty.getValue());
+        BibtexStringViewModel toBeRemoved = selectedItemProperty.getValue();
+        allStrings.remove(toBeRemoved);
     }
 
     private BibtexStringViewModel convertFromBibTexString(BibtexString bibtexString) {
@@ -72,14 +71,13 @@ public ObjectProperty<BibtexStringViewModel> seletedItemProperty() {
 
     public void save() {
         List<BibtexString> stringsToAdd = allStrings.stream().map(this::fromBibtexStringViewModel).collect(Collectors.toList());
-        bibDatabase.addStrings(stringsToAdd);
+        bibDatabase.setStrings(stringsToAdd);
     }
 
     private BibtexString fromBibtexStringViewModel(BibtexStringViewModel viewModel) {
         String label = viewModel.labelProperty().getValue();
         String content = viewModel.contentProperty().getValue();
         return new BibtexString(label, content);
-
     }
 
     public BooleanProperty validProperty() {

src/main/java/org/jabref/model/database/BibDatabase.java

@@ -49,7 +49,7 @@ public class BibDatabase {
      * State attributes
      */
     private final ObservableList<BibEntry> entries = FXCollections.synchronizedObservableList(FXCollections.observableArrayList(BibEntry::getObservables));
-    private final Map<String, BibtexString> bibtexStrings = new ConcurrentHashMap<>();
+    private Map<String, BibtexString> bibtexStrings = new ConcurrentHashMap<>();
     /**
      * this is kept in sync with the database (upon adding/removing an entry, it is updated as well)
      */
@@ -292,15 +292,14 @@ public synchronized void addString(BibtexString string) throws KeyCollisionExcep
         bibtexStrings.put(string.getId(), string);
     }
 
-    public void addStrings(Collection<BibtexString> stringsToAdd) {
-        for (BibtexString str : stringsToAdd) {
-            Optional<BibtexString> bibtexString = getStringByName(str.getName());
-            if (bibtexString.isPresent() && !(bibtexString.get().getContent().equals(str.getContent()))) {
-                bibtexString.get().setContent(str.getContent());
-            } else {
-                addString(str);
-            }
-        }
+    /**
+     * Replaces the existing lists of BibTexString with the given one
+     * No Duplicate checks are performed
+     * @param stringsToAdd The collection of strings to set
+     */
+    public void setStrings(Collection<BibtexString> stringsToAdd) {
+        Map<String, BibtexString> strs = stringsToAdd.stream().collect(Collectors.toConcurrentMap(BibtexString::getId, (bibtexStr) -> bibtexStr));
+        bibtexStrings = strs;
     }
 
     /**

src/main/resources/l10n/JabRef_en.properties

@@ -2092,4 +2092,7 @@ Set\ rank\ to\ three=Set rank to three
 Set\ rank\ to\ four=Set rank to four
 Set\ rank\ to\ five=Set rank to five
 
-Executing\ command\ "%0"...=Executing command "%0"...
+A\ string\ with\ the\ label\ '%0'\ already\ exists.=A string with the label '%0' already exists.
+
+Executing\ command\ "%0"...=Executing command "%0"...
+

src/test/java/org/jabref/model/database/BibDatabaseTest.java

@@ -123,27 +123,28 @@ public void hasStringLabelFindsString() {
     }
 
     @Test
-    public void addStringAsCollection() {
+    public void setSingleStringAsCollection() {
         BibtexString string = new BibtexString("DSP", "Digital Signal Processing");
         List<BibtexString> strings = Arrays.asList(string);
-        database.addStrings(strings);
+        database.setStrings(strings);
         assertEquals(Optional.of(string), database.getStringByName("DSP"));
     }
 
     @Test
-    public void addStringAsCollectionWithUpdatedContent() {
+    public void setStringAsCollectionWithUpdatedContentOverridesString() {
         BibtexString string = new BibtexString("DSP", "Digital Signal Processing");
-        List<BibtexString> strings = Arrays.asList(string, new BibtexString("DSP", "ABCD"));
-        database.addStrings(strings);
-        assertEquals(Optional.of(string), database.getStringByName("DSP"));
+        BibtexString newContent = new BibtexString("DSP", "ABCD");
+        List<BibtexString> strings = Arrays.asList(string, newContent);
+        database.setStrings(strings);
+        assertEquals(Optional.of(newContent), database.getStringByName("DSP"));
     }
 
     @Test
-    public void addStringAsCollectionWithNewContent() {
+    public void setStringAsCollectionWithNewContent() {
         BibtexString string = new BibtexString("DSP", "Digital Signal Processing");
         BibtexString vlsi = new BibtexString("VLSI", "Very Large Scale Integration");
         List<BibtexString> strings = Arrays.asList(string, vlsi);
-        database.addStrings(strings);
+        database.setStrings(strings);
         assertEquals(Optional.of(string), database.getStringByName("DSP"));
         assertEquals(Optional.of(vlsi), database.getStringByName("VLSI"));
     }