Fix #1873 group name matching

JabRef · Dec 10, 2016 · cf5224b · cf5224b
1 parent 4533e73
commit cf5224b
Show file tree

Hide file tree

Showing 13 changed files with 57 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -28,6 +28,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 ### Fixed
 - We fixed a few groups related issues:
   - "Remove entries from group" no longer removes entries from groups with similar names. [#2334](https://github.com/JabRef/jabref/issues/2334)
+  - If an entry's group field contains 'a b' it is no longer considered a member the groups 'a', 'b', and 'a b'. [1873](https://github.com/JabRef/jabref/issues/1873)
 - We fixed an issue which prevented JabRef from closing using the "Quit" menu command. [#2336](https://github.com/JabRef/jabref/issues/2336)
 - We fixed an issue where the file permissions of the .bib-file were changed upon saving [#2279](https://github.com/JabRef/jabref/issues/2279).
 - We fixed an issue which prevented that a database was saved successfully if JabRef failed to generate new BibTeX-keys [#2285](https://github.com/JabRef/jabref/issues/2285).

diff --git a/src/jmh/java/net/sf/jabref/benchmarks/Benchmarks.java b/src/jmh/java/net/sf/jabref/benchmarks/Benchmarks.java
@@ -125,7 +125,7 @@ public String htmlToLatexConversion() {
 
     @Benchmark
     public boolean keywordGroupContains() throws ParseException {
-        KeywordGroup group = new SimpleKeywordGroup("testGroup", GroupHierarchyType.INDEPENDENT, "keyword", "testkeyword", false, ',');
+        KeywordGroup group = new SimpleKeywordGroup("testGroup", GroupHierarchyType.INDEPENDENT, "keyword", "testkeyword", false, ',', false);
         return group.containsAll(database.getEntries());
     }
 

diff --git a/src/main/java/net/sf/jabref/gui/groups/AutoGroupDialog.java b/src/main/java/net/sf/jabref/gui/groups/AutoGroupDialog.java
@@ -115,7 +115,7 @@ public AutoGroupDialog(JabRefFrame jabrefFrame, BasePanel basePanel,
 
                 for (String keyword : keywords) {
                     SimpleKeywordGroup group = new SimpleKeywordGroup(
-                            formatter.format(keyword), GroupHierarchyType.INDEPENDENT, fieldText, keyword, false, Globals.prefs.getKeywordDelimiter());
+                            formatter.format(keyword), GroupHierarchyType.INDEPENDENT, fieldText, keyword, false, Globals.prefs.getKeywordDelimiter(), false);
                     autoGroupsRoot.addChild(GroupTreeNode.fromGroup(group));
                 }
 

diff --git a/src/main/java/net/sf/jabref/gui/groups/GroupDialog.java b/src/main/java/net/sf/jabref/gui/groups/GroupDialog.java
@@ -272,7 +272,7 @@ public void actionPerformed(ActionEvent e) {
                     } else {
                         resultingGroup = new SimpleKeywordGroup(nameField.getText().trim(), getContext(),
                                 keywordGroupSearchField.getText().trim(), keywordGroupSearchTerm.getText().trim(),
-                                keywordGroupCaseSensitive.isSelected(), Globals.prefs.getKeywordDelimiter());
+                                keywordGroupCaseSensitive.isSelected(), Globals.prefs.getKeywordDelimiter(), false);
                     }
                 } else if (searchRadioButton.isSelected()) {
                     try {

diff --git a/src/main/java/net/sf/jabref/logic/importer/util/GroupsParser.java b/src/main/java/net/sf/jabref/logic/importer/util/GroupsParser.java
@@ -109,7 +109,7 @@ private static KeywordGroup keywordGroupFromString(String s, Character keywordSe
         if (regExp) {
             return new RegexKeywordGroup(name, context, field, expression, caseSensitive);
         } else {
-            return new SimpleKeywordGroup(name, context, field, expression, caseSensitive, keywordSeparator);
+            return new SimpleKeywordGroup(name, context, field, expression, caseSensitive, keywordSeparator, false);
         }
     }
 

diff --git a/src/main/java/net/sf/jabref/model/entry/KeywordList.java b/src/main/java/net/sf/jabref/model/entry/KeywordList.java
@@ -7,6 +7,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -166,6 +167,10 @@ public String toString() {
         return getAsString(',');
     }
 
+    public Set<String> toStringList() {
+        return keywords.stream().map(Keyword::toString).collect(Collectors.toSet());
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {

diff --git a/src/main/java/net/sf/jabref/model/groups/ExplicitGroup.java b/src/main/java/net/sf/jabref/model/groups/ExplicitGroup.java
@@ -22,7 +22,7 @@ public class ExplicitGroup extends SimpleKeywordGroup {
     private final List<String> legacyEntryKeys = new ArrayList<>();
 
     public ExplicitGroup(String name, GroupHierarchyType context, Character keywordSeparator) {
-        super(name, context, FieldName.GROUPS, name, true, keywordSeparator);
+        super(name, context, FieldName.GROUPS, name, true, keywordSeparator, true);
     }
 
     public void addLegacyEntryKey(String key) {

diff --git a/src/main/java/net/sf/jabref/model/groups/SimpleKeywordGroup.java b/src/main/java/net/sf/jabref/model/groups/SimpleKeywordGroup.java
@@ -1,6 +1,7 @@
 package net.sf.jabref.model.groups;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
@@ -17,13 +18,16 @@ public class SimpleKeywordGroup extends KeywordGroup implements GroupEntryChange
 
     protected final Character keywordSeparator;
     private final List<String> searchWords;
+    private final boolean onlySplitWordsAtSeparator;
 
     public SimpleKeywordGroup(String name, GroupHierarchyType context, String searchField,
-                              String searchExpression, boolean caseSensitive, Character keywordSeparator) {
+                              String searchExpression, boolean caseSensitive, Character keywordSeparator,
+                              boolean onlySplitWordsAtSeparator) {
         super(name, context, searchField, searchExpression, caseSensitive);
 
         this.keywordSeparator = keywordSeparator;
         this.searchWords = StringUtil.getStringAsWords(searchExpression);
+        this.onlySplitWordsAtSeparator = onlySplitWordsAtSeparator;
     }
 
     private static boolean containsCaseInsensitive(Set<String> searchIn, List<String> searchFor) {
@@ -91,7 +95,8 @@ public boolean equals(Object o) {
                 && searchField.equals(other.searchField)
                 && searchExpression.equals(other.searchExpression)
                 && (caseSensitive == other.caseSensitive)
-                && keywordSeparator == other.keywordSeparator;
+                && keywordSeparator == other.keywordSeparator
+                && onlySplitWordsAtSeparator == other.onlySplitWordsAtSeparator;
     }
 
     @Override
@@ -105,13 +110,19 @@ public boolean contains(BibEntry entry) {
     }
 
     private Set<String> getFieldContentAsWords(BibEntry entry) {
-        return entry.getFieldAsWords(searchField);
+        if (onlySplitWordsAtSeparator) {
+            return entry.getField(searchField)
+                    .map(content -> KeywordList.parse(content, keywordSeparator).toStringList())
+                    .orElse(Collections.emptySet());
+        } else {
+            return entry.getFieldAsWords(searchField);
+        }
     }
 
     @Override
     public AbstractGroup deepCopy() {
         return new SimpleKeywordGroup(getName(), getHierarchicalContext(), searchField, searchExpression,
-                caseSensitive, keywordSeparator);
+                caseSensitive, keywordSeparator, onlySplitWordsAtSeparator);
     }
 
     @Override
@@ -121,6 +132,7 @@ public int hashCode() {
                 searchField,
                 searchExpression,
                 caseSensitive,
-                keywordSeparator);
+                keywordSeparator,
+                onlySplitWordsAtSeparator);
     }
 }
diff --git a/src/test/java/net/sf/jabref/logic/exporter/GroupSerializerTest.java b/src/test/java/net/sf/jabref/logic/exporter/GroupSerializerTest.java
@@ -15,7 +15,6 @@
 import net.sf.jabref.model.groups.SimpleKeywordGroup;
 
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -45,7 +44,7 @@ public void serializeSingleExplicitGroup() {
 
     @Test
     public void serializeSingleSimpleKeywordGroup() {
-        SimpleKeywordGroup group = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", false, ',');
+        SimpleKeywordGroup group = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", false, ',', false);
         List<String> serialization = groupSerializer.serializeTree(GroupTreeNode.fromGroup(group));
         assertEquals(Collections.singletonList("0 KeywordGroup:name;0;keywords;test;0;0;"), serialization);
     }

diff --git a/src/test/java/net/sf/jabref/logic/importer/fileformat/BibtexParserTest.java b/src/test/java/net/sf/jabref/logic/importer/fileformat/BibtexParserTest.java
@@ -1556,7 +1556,7 @@ public void integrationTestGroupTree() throws IOException, ParseException {
                 new RegexKeywordGroup("Fréchet", GroupHierarchyType.INDEPENDENT, "keywords", "FrechetSpace", false),
                 root.getChildren().get(0).getGroup());
         assertEquals(
-                new SimpleKeywordGroup("Invariant theory", GroupHierarchyType.INDEPENDENT, "keywords", "GIT", false, ','),
+                new SimpleKeywordGroup("Invariant theory", GroupHierarchyType.INDEPENDENT, "keywords", "GIT", false, ',', false),
                 root.getChildren().get(1).getGroup());
         assertEquals(Arrays.asList("Key1", "Key2"),
                 ((ExplicitGroup) root.getChildren().get(2).getGroup()).getLegacyEntryKeys());

diff --git a/src/test/java/net/sf/jabref/model/groups/ExplicitGroupTest.java b/src/test/java/net/sf/jabref/model/groups/ExplicitGroupTest.java
@@ -9,6 +9,7 @@
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 
 public class ExplicitGroupTest {
 
@@ -72,4 +73,20 @@ public void removeDoesNotChangeFieldIfContainsNameAsWord() throws Exception {
 
         assertEquals(Optional.of("myExplicitGroup alternative"), entry.getField(FieldName.GROUPS));
     }
+
+    @Test
+    // For https://github.com/JabRef/jabref/issues/1873
+    public void containsOnlyMatchesCompletePhraseWithWhitespace() throws Exception {
+        entry.setField(FieldName.GROUPS, "myExplicitGroup b");
+
+        assertFalse(group.contains(entry));
+    }
+
+    @Test
+    // For https://github.com/JabRef/jabref/issues/1873
+    public void containsOnlyMatchesCompletePhraseWithSlash() throws Exception {
+        entry.setField(FieldName.GROUPS, "myExplicitGroup/b");
+
+        assertFalse(group.contains(entry));
+    }
 }
diff --git a/src/test/java/net/sf/jabref/model/groups/GroupTreeNodeTest.java b/src/test/java/net/sf/jabref/model/groups/GroupTreeNodeTest.java
@@ -87,7 +87,7 @@ public static GroupTreeNode getNodeInComplexTree(GroupTreeNode root) {
     }
 
     private static AbstractGroup getKeywordGroup(String name) {
-        return new SimpleKeywordGroup(name, GroupHierarchyType.INDEPENDENT, "searchField", "searchExpression", true,',');
+        return new SimpleKeywordGroup(name, GroupHierarchyType.INDEPENDENT, "searchField", "searchExpression", true,',', false);
     }
 
     private static AbstractGroup getSearchGroup(String name) {
@@ -167,35 +167,35 @@ public void numberOfHitsReturnsZeroForEmptyList() throws Exception {
     public void numberOfHitsMatchesOneEntry() throws Exception {
         GroupTreeNode parent = getNodeInSimpleTree();
         GroupTreeNode node = parent.addSubgroup(
-                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author2", true, ','));
+                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author2", true, ',', false));
         assertEquals(1, node.numberOfMatches(entries));
     }
 
     @Test
     public void numberOfHitsMatchesMultipleEntries() throws Exception {
         GroupTreeNode parent = getNodeInSimpleTree();
         GroupTreeNode node = parent.addSubgroup(
-                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author1", true, ','));
+                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author1", true, ',', false));
         assertEquals(2, node.numberOfMatches(entries));
     }
 
     @Test
     public void numberOfHitsWorksForRefiningGroups() throws Exception {
         GroupTreeNode grandParent = getNodeInSimpleTree();
         GroupTreeNode parent = grandParent.addSubgroup(
-                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author2", true, ','));
+                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author2", true, ',', false));
         GroupTreeNode node = parent.addSubgroup(
-                new SimpleKeywordGroup("node", GroupHierarchyType.REFINING, "author", "author1", true, ','));
+                new SimpleKeywordGroup("node", GroupHierarchyType.REFINING, "author", "author1", true, ',', false));
         assertEquals(1, node.numberOfMatches(entries));
     }
 
     @Test
     public void numberOfHitsWorksForHierarchyOfIndependentGroups() throws Exception {
         GroupTreeNode grandParent = getNodeInSimpleTree();
         GroupTreeNode parent = grandParent.addSubgroup(
-                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author2", true, ','));
+                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author2", true, ',', false));
         GroupTreeNode node = parent.addSubgroup(
-                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author1", true, ','));
+                new SimpleKeywordGroup("node", GroupHierarchyType.INDEPENDENT, "author", "author1", true, ',', false));
         assertEquals(2, node.numberOfMatches(entries));
     }
 

diff --git a/src/test/java/net/sf/jabref/model/groups/SimpleKeywordGroupTest.java b/src/test/java/net/sf/jabref/model/groups/SimpleKeywordGroupTest.java
@@ -21,9 +21,9 @@ public class SimpleKeywordGroupTest {
 
     @Before
     public void setUp() {
-        testGroup = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", false, ',');
-        testCaseSensitiveGroup = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", true, ',');
-        waterGroup = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "\\H2O", false, ',');
+        testGroup = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", false, ',', false);
+        testCaseSensitiveGroup = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "test", true, ',', false);
+        waterGroup = new SimpleKeywordGroup("name", GroupHierarchyType.INDEPENDENT, "keywords", "\\H2O", false, ',', false);
         entry = new BibEntry();
     }