Keyword - Special field synchronization (#2583)

* Fixes #2580 Keyword - Special field synchronization * Changelog
JabRef · Feb 22, 2017 · 1aa1ad0 · 1aa1ad0
1 parent bdc535f
commit 1aa1ad0
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 36 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,7 +45,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
  - We fixed the import of MS-Office XML files, when the `month` field contained an invalid value.
  - ArXiV fetcher now checks similarity of entry when using DOI retrieval to avoid false positives [#2575](https://github.com/JabRef/jabref/issues/2575)
  - Sciencedirect/Elsevier fetcher is now able to scrape new HTML structure [#2576](https://github.com/JabRef/jabref/issues/2576)
-
+ - Fixed the synchronization logic of keywords and special fields and vice versa [#2580](https://github.com/JabRef/jabref/issues/2580)
 
 
 ### Removed

diff --git a/src/main/java/org/jabref/gui/FileDialog.java b/src/main/java/org/jabref/gui/FileDialog.java
@@ -35,7 +35,6 @@
  */
 @Deprecated
 public class FileDialog {
-
     private static final Log LOGGER = LogFactory.getLog(FileDialog.class);
 
     private final FileChooser fileChooser;

diff --git a/src/main/java/org/jabref/gui/specialfields/SpecialFieldDatabaseChangeListener.java b/src/main/java/org/jabref/gui/specialfields/SpecialFieldDatabaseChangeListener.java
@@ -26,19 +26,19 @@ public static SpecialFieldDatabaseChangeListener getInstance() {
 
     @Subscribe
     public void listen(EntryAddedEvent event) {
-        if (Globals.prefs.isKeywordSyncEnabled()) {
-            final BibEntry entry = event.getBibEntry();
-            // NamedCompount code similar to SpecialFieldUpdateListener
-            NamedCompound nc = new NamedCompound(Localization.lang("Synchronized special fields based on keywords"));
-            List<FieldChange> changes = SpecialFieldsUtils.syncSpecialFieldsFromKeywords(entry, Globals.prefs.getKeywordDelimiter());
-            for(FieldChange change: changes) {
-                nc.addEdit(new UndoableFieldChange(change));
-            }
-
-            // Don't insert the compound into the undoManager,
-            // it would be added before the component which undoes the insertion of the entry and creates heavy problems
-            // (which prohibits the undo the deleting multiple entries)
+        if (!Globals.prefs.isKeywordSyncEnabled()) {
+            return;
         }
-    }
 
+        final BibEntry entry = event.getBibEntry();
+        // NamedCompount code similar to SpecialFieldUpdateListener
+        NamedCompound nc = new NamedCompound(Localization.lang("Synchronized special fields based on keywords"));
+        List<FieldChange> changes = SpecialFieldsUtils.syncSpecialFieldsFromKeywords(entry, Globals.prefs.getKeywordDelimiter());
+        for(FieldChange change: changes) {
+            nc.addEdit(new UndoableFieldChange(change));
+        }
+        // Don't insert the compound into the undoManager,
+        // it would be added before the component which undoes the insertion of the entry and creates heavy problems
+        // (which prohibits the undo the deleting multiple entries)
+    }
 }
diff --git a/src/main/java/org/jabref/gui/specialfields/SpecialFieldUpdateListener.java b/src/main/java/org/jabref/gui/specialfields/SpecialFieldUpdateListener.java
@@ -23,6 +23,11 @@ public class SpecialFieldUpdateListener {
 
     @Subscribe
     public void listen(FieldChangedEvent fieldChangedEvent) {
+        // only sync if keyword sync is enabled
+        if (!Globals.prefs.isKeywordSyncEnabled()) {
+            return;
+        }
+
         final BibEntry entry = fieldChangedEvent.getBibEntry();
         final String fieldName = fieldChangedEvent.getFieldName();
         // Source editor cycles through all entries
@@ -32,15 +37,10 @@ public void listen(FieldChangedEvent fieldChangedEvent) {
         SwingUtilities.invokeLater(() -> {
             if (FieldName.KEYWORDS.equals(fieldName)) {
                 SpecialFieldsUtils.syncSpecialFieldsFromKeywords(entry, Globals.prefs.getKeywordDelimiter());
-                SwingUtilities
-                        .invokeLater(() -> JabRefGUI.getMainFrame().getCurrentBasePanel().updateEntryEditorIfShowing());
-            } else {
-                if (SpecialField.isSpecialField(fieldName)) {
-                    SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, Globals.prefs.isKeywordSyncEnabled(), Globals.prefs.getKeywordDelimiter());
-                    SwingUtilities.invokeLater(
-                            () -> JabRefGUI.getMainFrame().getCurrentBasePanel().updateEntryEditorIfShowing());
-                }
+            } else if (SpecialField.isSpecialField(fieldName)) {
+                SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, Globals.prefs.getKeywordDelimiter());
             }
+            SwingUtilities.invokeLater(() -> JabRefGUI.getMainFrame().getCurrentBasePanel().updateEntryEditorIfShowing());
         });
     }
 

diff --git a/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java b/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java
@@ -35,18 +35,16 @@ public static List<FieldChange> updateField(SpecialField field, String value, Bi
         UpdateField.updateField(entry, field.getFieldName(), value, nullFieldIfValueIsTheSame)
                 .ifPresent(fieldChange -> fieldChanges.add(fieldChange));
         // we cannot use "value" here as updateField has side effects: "nullFieldIfValueIsTheSame" nulls the field if value is the same
-        fieldChanges.addAll(SpecialFieldsUtils.exportFieldToKeywords(field, entry, isKeywordSyncEnabled, keywordDelimiter));
+        if (isKeywordSyncEnabled) {
+            fieldChanges.addAll(SpecialFieldsUtils.exportFieldToKeywords(field, entry, keywordDelimiter));
+        }
 
         return fieldChanges;
     }
 
-    private static List<FieldChange> exportFieldToKeywords(SpecialField specialField, BibEntry entry, boolean isKeywordSyncEnabled, Character keywordDelimiter) {
+    private static List<FieldChange> exportFieldToKeywords(SpecialField specialField, BibEntry entry, Character keywordDelimiter) {
         List<FieldChange> fieldChanges = new ArrayList<>();
 
-        if (!isKeywordSyncEnabled) {
-            return fieldChanges;
-        }
-
         Optional<Keyword> newValue = entry.getField(specialField.getFieldName()).map(Keyword::new);
         KeywordList keyWords = specialField.getKeyWords();
 
@@ -59,18 +57,19 @@ private static List<FieldChange> exportFieldToKeywords(SpecialField specialField
     /**
      * Update keywords according to values of special fields
      */
-    public static List<FieldChange> syncKeywordsFromSpecialFields(BibEntry entry, boolean isKeywordSyncEnabled, Character keywordDelimiter) {
+    public static List<FieldChange> syncKeywordsFromSpecialFields(BibEntry entry, Character keywordDelimiter) {
         List<FieldChange> fieldChanges = new ArrayList<>();
 
         for(SpecialField field: SpecialField.values()) {
-            fieldChanges.addAll(SpecialFieldsUtils.exportFieldToKeywords(field, entry, isKeywordSyncEnabled, keywordDelimiter));
+            fieldChanges.addAll(SpecialFieldsUtils.exportFieldToKeywords(field, entry, keywordDelimiter));
         }
 
         return fieldChanges;
     }
 
     private static List<FieldChange> importKeywordsForField(KeywordList keywordList, SpecialField field, BibEntry entry) {
         List<FieldChange> fieldChanges = new ArrayList<>();
+
         KeywordList values = field.getKeyWords();
         Optional<Keyword> newValue = Optional.empty();
         for (Keyword keyword : values) {
@@ -88,7 +87,7 @@ private static List<FieldChange> importKeywordsForField(KeywordList keywordList,
     }
 
     /**
-     * updates field values according to keywords
+     * Updates special field values according to keywords
      */
     public static List<FieldChange> syncSpecialFieldsFromKeywords(BibEntry entry, Character keywordDelimiter) {
         List<FieldChange> fieldChanges = new ArrayList<>();

diff --git a/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java b/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java
@@ -19,15 +19,15 @@ public class SpecialFieldsUtilsTest {
     public void syncKeywordsFromSpecialFieldsWritesToKeywords() {
         BibEntry entry = new BibEntry();
         entry.setField("ranking", "rank2");
-        SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, true, ',');
+        SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, ',');
         assertEquals(Optional.of("rank2"), entry.getField("keywords"));
     }
 
     @Test
     public void syncKeywordsFromSpecialFieldsCausesChange() {
         BibEntry entry = new BibEntry();
         entry.setField("ranking", "rank2");
-        List<FieldChange> changes = SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, true, ',');
+        List<FieldChange> changes = SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, ',');
         assertTrue(changes.size() > 0);
     }
 
@@ -36,14 +36,14 @@ public void syncKeywordsFromSpecialFieldsOverwritesKeywords() {
         BibEntry entry = new BibEntry();
         entry.setField("ranking", "rank2");
         entry.setField("keywords", "rank3");
-        SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, true, ',');
+        SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, ',');
         assertEquals(Optional.of("rank2"), entry.getField("keywords"));
     }
 
     @Test
     public void syncKeywordsFromSpecialFieldsForEmptyFieldCausesNoChange() {
         BibEntry entry = new BibEntry();
-        List<FieldChange> changes = SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, true, ',');
+        List<FieldChange> changes = SpecialFieldsUtils.syncKeywordsFromSpecialFields(entry, ',');
         assertFalse(changes.size() > 0);
     }