Fix the removal of special fields when using keyword sync

Fixes JabRef#3432
derTimme · Nov 17, 2017 · ac56a38 · ac56a38
1 parent 8d36fae
commit ac56a38
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 3 deletions.
diff --git a/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java b/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java
@@ -46,9 +46,12 @@ private static List<FieldChange> exportFieldToKeywords(SpecialField specialField
 
         Optional<Keyword> newValue = entry.getField(specialField.getFieldName()).map(Keyword::new);
         KeywordList keyWords = specialField.getKeyWords();
-
-        Optional<FieldChange> change = entry.replaceKeywords(keyWords, newValue, keywordDelimiter);
-        change.ifPresent(changeValue -> fieldChanges.add(changeValue));
+        if(newValue.isPresent()) {
+            Optional<FieldChange> change = entry.replaceKeywords(keyWords, newValue, keywordDelimiter);
+            change.ifPresent(changeValue -> fieldChanges.add(changeValue));
+        } else {
+            entry.removeKeywords(keyWords, keywordDelimiter).ifPresent(changeValue -> fieldChanges.add(changeValue));
+        }
 
         return fieldChanges;
     }

diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java
@@ -678,6 +678,12 @@ public KeywordList getResolvedKeywords(Character delimiter, BibDatabase database
         return KeywordList.parse(keywordsContent, delimiter);
     }
 
+    public Optional<FieldChange> removeKeywords(KeywordList keywordsToRemove, Character keywordDelimiter) {
+        KeywordList keywordList = getKeywords(keywordDelimiter);
+        keywordList.removeAll(keywordsToRemove);
+        return putKeywords(keywordList, keywordDelimiter);
+    }
+
     public Optional<FieldChange> replaceKeywords(KeywordList keywordsToReplace, Optional<Keyword> newValue,
                                                  Character keywordDelimiter) {
         KeywordList keywordList = getKeywords(keywordDelimiter);

diff --git a/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java b/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java
@@ -6,6 +6,9 @@
 import org.jabref.model.FieldChange;
 import org.jabref.model.entry.BibEntry;
 
+import org.jabref.model.entry.Keyword;
+import org.jabref.model.entry.KeywordList;
+import org.jabref.model.entry.specialfields.SpecialField;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -69,4 +72,18 @@ public void syncSpecialFieldsFromKeywordCausesNoChangeWhenKeywordsAreEmpty() {
         List<FieldChange> changes = SpecialFieldsUtils.syncSpecialFieldsFromKeywords(entry, ',');
         assertFalse(changes.size() > 0);
     }
+
+    @Test
+    public void updateFieldRemovesSpecialFieldKeywordWhenKeywordSyncIsUsed() {
+        BibEntry entry = new BibEntry();
+        SpecialField specialField = SpecialField.PRINTED;
+        Keyword specialFieldKeyword = specialField.getKeyWords().get(0);
+        // Add the special field
+        SpecialFieldsUtils.updateField(specialField, specialFieldKeyword.get(), entry, true, true, ',');
+        // Remove it
+        List<FieldChange> changes = SpecialFieldsUtils.updateField(specialField, specialFieldKeyword.get(), entry, true, true, ',');
+        assertTrue(changes.size() > 0);
+        KeywordList remainingKeywords = entry.getKeywords(',');
+        assertFalse(remainingKeywords.contains(specialFieldKeyword));
+    }
 }
diff --git a/src/test/java/org/jabref/model/entry/BibEntryTests.java b/src/test/java/org/jabref/model/entry/BibEntryTests.java
@@ -8,11 +8,14 @@
 
 import org.jabref.model.FieldChange;
 
+import org.jabref.model.entry.specialfields.SpecialField;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 
 public class BibEntryTests {
 
@@ -369,6 +372,36 @@ public void getKeywordsReturnsParsedKeywordListFromKeywordsField() {
         assertEquals(new KeywordList("w1", "w2a w2b", "w3"), entry.getKeywords(','));
     }
 
+    @Test
+    public void removeKeywordsOnEntryWithoutKeywordsDoesNothing() {
+        BibEntry entry = new BibEntry();
+        Optional<FieldChange> change = entry.removeKeywords(SpecialField.RANKING.getKeyWords(), ',');
+        assertEquals(Optional.empty(), change);
+    }
+
+    @Test
+    public void removeKeywordsWithEmptyListDoesNothing() {
+        keywordEntry.putKeywords(Arrays.asList("kw1", "kw2"), ',');
+        Optional<FieldChange> change = keywordEntry.removeKeywords(new KeywordList(), ',');
+        assertEquals(Optional.empty(), change);
+    }
+
+    @Test
+    public void removeKeywordsWithNonExistingKeywordsDoesNothing() {
+        keywordEntry.putKeywords(Arrays.asList("kw1", "kw2"), ',');
+        Optional<FieldChange> change = keywordEntry.removeKeywords(KeywordList.parse("kw3, kw4", ','), ',');
+        assertEquals(Optional.empty(), change);
+        assertEquals(2, keywordEntry.getKeywords(',').size());
+    }
+
+    @Test
+    public void removeKeywordsWithExistingKeywordsRemovesThem() {
+        keywordEntry.putKeywords(Arrays.asList("kw1", "kw2", "kw3"), ',');
+        Optional<FieldChange> change = keywordEntry.removeKeywords(KeywordList.parse("kw1, kw2", ','), ',');
+        assertTrue(change.isPresent());
+        assertEquals(KeywordList.parse("kw3", ','), keywordEntry.getKeywords(','));
+    }
+
     @Test
     public void testGroupAndSearchHits() {
         BibEntry be = new BibEntry();