From fa7763bc3269a1dbebf0c0347fcafe203dcac41e Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Thu, 25 Aug 2016 08:14:48 +0200 Subject: [PATCH] Fixes #1687 "month" field ascending/descending sorting swapped (#1837) --- CHANGELOG.md | 1 + .../bibtex/comparator/FieldComparator.java | 51 ++--- .../comparator/FieldComparatorTest.java | 182 ++++++++++++++++++ 3 files changed, 199 insertions(+), 35 deletions(-) create mode 100644 src/test/java/net/sf/jabref/logic/bibtex/comparator/FieldComparatorTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index fc4b5cf07742..c6a6496f8a34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - Fixed [#1531](https://github.com/JabRef/jabref/issues/1531): `\relax` can be used for abbreviation of author names - Fixed [#1771](https://github.com/JabRef/jabref/issues/1771): Show all supported import types as default - Fixed [#1804](https://github.com/JabRef/jabref/issues/1804): Integrity check no longer removes URL field by mistake +- Fixed [#1687](https://github.com/JabRef/jabref/issues/1687): "month" field ascending/descending sorting swapped diff --git a/src/main/java/net/sf/jabref/logic/bibtex/comparator/FieldComparator.java b/src/main/java/net/sf/jabref/logic/bibtex/comparator/FieldComparator.java index e00cd1768eaf..beb1af70a9a2 100644 --- a/src/main/java/net/sf/jabref/logic/bibtex/comparator/FieldComparator.java +++ b/src/main/java/net/sf/jabref/logic/bibtex/comparator/FieldComparator.java @@ -18,16 +18,7 @@ import net.sf.jabref.model.entry.MonthUtil; /** - * * A comparator for BibEntry fields - * - * Initial Version: - * - * @author alver - * @version Date: Oct 13, 2005 Time: 10:10:04 PM To - * - * TODO: Testcases - * */ public class FieldComparator implements Comparator { @@ -47,26 +38,16 @@ public FieldComparator(String field) { this(field, false); } - public FieldComparator(String field, boolean reversed) { + public FieldComparator(SaveOrderConfig.SortCriterion sortCriterion) { + this(sortCriterion.field, sortCriterion.descending); + } + + public FieldComparator(String field, boolean descending) { this.fieldName = Objects.requireNonNull(field); this.field = fieldName.split(FieldName.FIELD_SEPARATOR); fieldType = determineFieldType(); isNumeric = InternalBibtexFields.isNumeric(this.field[0]); - - if(fieldType == FieldType.MONTH) { - /* - * [ 1598777 ] Month sorting - * - * http://sourceforge.net/tracker/index.php?func=detail&aid=1598777&group_id=92314&atid=600306 - */ - multiplier = reversed ? 1 : -1; - } else { - multiplier = reversed ? -1 : 1; - } - } - - public FieldComparator(SaveOrderConfig.SortCriterion sortCriterion) { - this(sortCriterion.field, sortCriterion.descending); + multiplier = descending ? -1 : 1; } private static Collator getCollator() { @@ -92,6 +73,16 @@ private FieldType determineFieldType() { } } + private String getField(BibEntry entry) { + for (String aField : field) { + Optional o = entry.getFieldOrAlias(aField); + if (o.isPresent()) { + return o.get(); + } + } + return null; + } + @Override public int compare(BibEntry e1, BibEntry e2) { String f1; @@ -154,16 +145,6 @@ public int compare(BibEntry e1, BibEntry e2) { return COLLATOR.compare(ours, theirs) * multiplier; } - private String getField(BibEntry entry) { - for (String aField : field) { - Optional o = entry.getFieldOrAlias(aField); - if (o.isPresent()) { - return o.get(); - } - } - return null; - } - /** * Returns the field this Comparator compares by. * diff --git a/src/test/java/net/sf/jabref/logic/bibtex/comparator/FieldComparatorTest.java b/src/test/java/net/sf/jabref/logic/bibtex/comparator/FieldComparatorTest.java new file mode 100644 index 000000000000..ceb3ef8d98ba --- /dev/null +++ b/src/test/java/net/sf/jabref/logic/bibtex/comparator/FieldComparatorTest.java @@ -0,0 +1,182 @@ +package net.sf.jabref.logic.bibtex.comparator; + +import net.sf.jabref.model.entry.BibEntry; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class FieldComparatorTest { + @Test + public void compareMonthFieldIdentity() throws Exception { + FieldComparator comparator = new FieldComparator("month"); + BibEntry equal = new BibEntry(); + equal.setField("month", "1"); + + assertEquals(0, comparator.compare(equal, equal)); + } + + @Test + public void compareMonthFieldEquality() throws Exception { + FieldComparator comparator = new FieldComparator("month"); + BibEntry equal = new BibEntry(); + equal.setField("month", "1"); + BibEntry equal2 = new BibEntry(); + equal2.setField("month", "1"); + + assertEquals(0, comparator.compare(equal, equal2)); + } + + @Test + public void compareMonthFieldBiggerAscending() throws Exception { + FieldComparator comparator = new FieldComparator("month"); + BibEntry smaller = new BibEntry(); + smaller.setField("month", "jan"); + BibEntry bigger = new BibEntry(); + bigger.setField("month", "feb"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareMonthFieldBiggerDescending() throws Exception { + FieldComparator comparator = new FieldComparator("month", true); + BibEntry smaller = new BibEntry(); + smaller.setField("month", "feb"); + BibEntry bigger = new BibEntry(); + bigger.setField("month", "jan"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareYearFieldIdentity() throws Exception { + FieldComparator comparator = new FieldComparator("year"); + BibEntry equal = new BibEntry(); + equal.setField("year", "2016"); + + assertEquals(0, comparator.compare(equal, equal)); + } + + @Test + public void compareYearFieldEquality() throws Exception { + FieldComparator comparator = new FieldComparator("year"); + BibEntry equal = new BibEntry(); + equal.setField("year", "2016"); + BibEntry equal2 = new BibEntry(); + equal2.setField("year", "2016"); + + assertEquals(0, comparator.compare(equal, equal2)); + } + + @Test + public void compareYearFieldBiggerAscending() throws Exception { + FieldComparator comparator = new FieldComparator("year"); + BibEntry smaller = new BibEntry(); + smaller.setField("year", "2000"); + BibEntry bigger = new BibEntry(); + bigger.setField("year", "2016"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareYearFieldBiggerDescending() throws Exception { + FieldComparator comparator = new FieldComparator("year", true); + BibEntry smaller = new BibEntry(); + smaller.setField("year", "2016"); + BibEntry bigger = new BibEntry(); + bigger.setField("year", "2000"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareTypeFieldIdentity() throws Exception { + FieldComparator comparator = new FieldComparator("entrytype"); + BibEntry equal = new BibEntry("1", "article"); + + assertEquals(0, comparator.compare(equal, equal)); + } + + @Test + public void compareTypeFieldEquality() throws Exception { + FieldComparator comparator = new FieldComparator("entrytype"); + BibEntry equal = new BibEntry("1", "article"); + BibEntry equal2 = new BibEntry("1", "article"); + + assertEquals(0, comparator.compare(equal, equal2)); + } + + @Test + public void compareTypeFieldBiggerAscending() throws Exception { + FieldComparator comparator = new FieldComparator("entrytype"); + BibEntry smaller = new BibEntry("1", "article"); + BibEntry bigger = new BibEntry("2", "book"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareTypeFieldBiggerDescending() throws Exception { + FieldComparator comparator = new FieldComparator("entrytype", true); + BibEntry bigger = new BibEntry("1", "article"); + BibEntry smaller = new BibEntry("2", "book"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareStringFieldsIdentity() throws Exception { + FieldComparator comparator = new FieldComparator("title"); + BibEntry equal = new BibEntry(); + equal.setField("title", "title"); + + assertEquals(0, comparator.compare(equal, equal)); + } + + @Test + public void compareStringFieldsEquality() throws Exception { + FieldComparator comparator = new FieldComparator("title"); + BibEntry equal = new BibEntry(); + equal.setField("title", "title"); + BibEntry equal2 = new BibEntry(); + equal2.setField("title", "title"); + + assertEquals(0, comparator.compare(equal, equal2)); + } + + @Test + public void compareStringFieldsBiggerAscending() throws Exception { + FieldComparator comparator = new FieldComparator("title"); + BibEntry bigger = new BibEntry(); + bigger.setField("title", "b"); + BibEntry smaller = new BibEntry(); + smaller.setField("title", "a"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void compareStringFieldsBiggerDescending() throws Exception { + FieldComparator comparator = new FieldComparator("title", true); + BibEntry bigger = new BibEntry(); + bigger.setField("title", "a"); + BibEntry smaller = new BibEntry(); + smaller.setField("title", "b"); + + assertEquals(1, comparator.compare(bigger, smaller)); + } + + @Test + public void nameOfComparisonField() throws Exception { + FieldComparator comparator = new FieldComparator("title"); + assertEquals("title", comparator.getFieldName()); + } + + @Test + public void nameOfComparisonFieldAlias() throws Exception { + FieldComparator comparator = new FieldComparator("author/editor"); + assertEquals("author/editor", comparator.getFieldName()); + } +}