diff --git a/AUTHORS b/AUTHORS index 596c7d7f254..32ec05421c7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -182,6 +182,7 @@ Thomas Arildsen Thomas Ilsche Thorsten Dahlheimer Tim van Rossum +Tim Würtele Tobias Boceck Tobias Denkinger Tobias Diez diff --git a/CHANGELOG.md b/CHANGELOG.md index ec471b44745..4760471a85a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - We fixed an issue where fetching entries from crossref that had no titles caused an error [#3376](https://github.com/JabRef/jabref/issues/3376) - We fixed an issue where the same Java Look and Feel would be listed more than once in the Preferences. [#3391](https://github.com/JabRef/jabref/issues/3391) - We fixed an issue where errors in citation styles triggered an exception when opening the preferences dialog [#3389](https://github.com/JabRef/jabref/issues/3389) + - We fixed an issue where special fields (such as `printed`) could not be cleared when syncing special fields via the keywords [#3432](https://github.com/JabRef/jabref/issues/3432) ### Removed diff --git a/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java b/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java index e5e1bf2764f..084239efe0d 100644 --- a/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java +++ b/src/main/java/org/jabref/logic/specialfields/SpecialFieldsUtils.java @@ -44,11 +44,11 @@ public static List updateField(SpecialField field, String value, Bi private static List exportFieldToKeywords(SpecialField specialField, BibEntry entry, Character keywordDelimiter) { List fieldChanges = new ArrayList<>(); - Optional newValue = entry.getField(specialField.getFieldName()).map(Keyword::new); KeywordList keyWords = specialField.getKeyWords(); - - Optional change = entry.replaceKeywords(keyWords, newValue, keywordDelimiter); - change.ifPresent(changeValue -> fieldChanges.add(changeValue)); + Optional newValue = entry.getField(specialField.getFieldName()).map(Keyword::new); + newValue.map(value -> entry.replaceKeywords(keyWords, newValue.get(), keywordDelimiter)) + .orElseGet(() -> entry.removeKeywords(keyWords, keywordDelimiter)) + .ifPresent(changeValue -> fieldChanges.add(changeValue)); return fieldChanges; } @@ -59,7 +59,7 @@ private static List exportFieldToKeywords(SpecialField specialField public static List syncKeywordsFromSpecialFields(BibEntry entry, Character keywordDelimiter) { List fieldChanges = new ArrayList<>(); - for (SpecialField field: SpecialField.values()) { + for (SpecialField field : SpecialField.values()) { fieldChanges.addAll(SpecialFieldsUtils.exportFieldToKeywords(field, entry, keywordDelimiter)); } @@ -96,7 +96,7 @@ public static List syncSpecialFieldsFromKeywords(BibEntry entry, Ch KeywordList keywordList = entry.getKeywords(keywordDelimiter); - for (SpecialField field: SpecialField.values()) { + for (SpecialField field : SpecialField.values()) { fieldChanges.addAll(SpecialFieldsUtils.importKeywordsForField(keywordList, field, entry)); } @@ -113,7 +113,7 @@ public static void synchronizeSpecialFields(KeywordList keywordsToAdd, KeywordLi // Priority clone = keywordsToAdd.createClone(); - for (SpecialField field: SpecialField.values()) { + for (SpecialField field : SpecialField.values()) { clone.retainAll(field.getKeyWords()); if (!clone.isEmpty()) { keywordsToRemove.addAll(field.getKeyWords()); diff --git a/src/main/java/org/jabref/model/entry/BibEntry.java b/src/main/java/org/jabref/model/entry/BibEntry.java index 644a3ddd2d5..54e8e0000ab 100644 --- a/src/main/java/org/jabref/model/entry/BibEntry.java +++ b/src/main/java/org/jabref/model/entry/BibEntry.java @@ -105,16 +105,6 @@ public Optional setMonth(Month parsedMonth) { return setField(FieldName.MONTH, parsedMonth.getJabRefFormat()); } - public Optional replaceKeywords(KeywordList keywordsToReplace, Optional newValue, - Character keywordDelimiter) { - KeywordList keywordList = getKeywords(keywordDelimiter); - if (newValue.isPresent()) { - keywordList.replaceAll(keywordsToReplace, newValue.get()); - } - - return putKeywords(keywordList, keywordDelimiter); - } - /** * Returns the text stored in the given field of the given bibtex entry * which belongs to the given database. @@ -688,6 +678,20 @@ public KeywordList getResolvedKeywords(Character delimiter, BibDatabase database return KeywordList.parse(keywordsContent, delimiter); } + public Optional removeKeywords(KeywordList keywordsToRemove, Character keywordDelimiter) { + KeywordList keywordList = getKeywords(keywordDelimiter); + keywordList.removeAll(keywordsToRemove); + return putKeywords(keywordList, keywordDelimiter); + } + + public Optional replaceKeywords(KeywordList keywordsToReplace, Keyword newValue, + Character keywordDelimiter) { + KeywordList keywordList = getKeywords(keywordDelimiter); + keywordList.replaceAll(keywordsToReplace, newValue); + + return putKeywords(keywordList, keywordDelimiter); + } + public Collection getFieldValues() { return fields.values(); } diff --git a/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java b/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java index 26ea98db537..792a09f3363 100644 --- a/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java +++ b/src/test/java/org/jabref/logic/specialfields/SpecialFieldsUtilsTest.java @@ -1,11 +1,16 @@ package org.jabref.logic.specialfields; +import java.util.Arrays; import java.util.List; import java.util.Optional; import org.jabref.model.FieldChange; import org.jabref.model.entry.BibEntry; +import org.jabref.model.entry.FieldName; +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 +74,19 @@ public void syncSpecialFieldsFromKeywordCausesNoChangeWhenKeywordsAreEmpty() { List 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 changes = SpecialFieldsUtils.updateField(specialField, specialFieldKeyword.get(), entry, true, true, ','); + assertEquals(Arrays.asList(new FieldChange(entry, specialField.getFieldName(), specialFieldKeyword.get(), null), + new FieldChange(entry, FieldName.KEYWORDS, specialFieldKeyword.get(), null)), changes); + 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 index b737b34cff2..c4bd826a453 100644 --- a/src/test/java/org/jabref/model/entry/BibEntryTests.java +++ b/src/test/java/org/jabref/model/entry/BibEntryTests.java @@ -6,13 +6,17 @@ import java.util.List; import java.util.Optional; +import com.google.common.collect.Sets; 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 +373,36 @@ public void getKeywordsReturnsParsedKeywordListFromKeywordsField() { assertEquals(new KeywordList("w1", "w2a w2b", "w3"), entry.getKeywords(',')); } + @Test + public void removeKeywordsOnEntryWithoutKeywordsDoesNothing() { + BibEntry entry = new BibEntry(); + Optional change = entry.removeKeywords(SpecialField.RANKING.getKeyWords(), ','); + assertEquals(Optional.empty(), change); + } + + @Test + public void removeKeywordsWithEmptyListDoesNothing() { + keywordEntry.putKeywords(Arrays.asList("kw1", "kw2"), ','); + Optional change = keywordEntry.removeKeywords(new KeywordList(), ','); + assertEquals(Optional.empty(), change); + } + + @Test + public void removeKeywordsWithNonExistingKeywordsDoesNothing() { + keywordEntry.putKeywords(Arrays.asList("kw1", "kw2"), ','); + Optional change = keywordEntry.removeKeywords(KeywordList.parse("kw3, kw4", ','), ','); + assertEquals(Optional.empty(), change); + assertEquals(Sets.newHashSet("kw1", "kw2"), keywordEntry.getKeywords(',').toStringList()); + } + + @Test + public void removeKeywordsWithExistingKeywordsRemovesThem() { + keywordEntry.putKeywords(Arrays.asList("kw1", "kw2", "kw3"), ','); + Optional change = keywordEntry.removeKeywords(KeywordList.parse("kw1, kw2", ','), ','); + assertTrue(change.isPresent()); + assertEquals(KeywordList.parse("kw3", ','), keywordEntry.getKeywords(',')); + } + @Test public void testGroupAndSearchHits() { BibEntry be = new BibEntry();