Fixes JabRef#1687 "month" field ascending/descending sorting swapped (J…

…abRef#1837)

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
 
 
 

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<BibEntry> {
 
@@ -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<String> 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<String> o = entry.getFieldOrAlias(aField);
-            if (o.isPresent()) {
-                return o.get();
-            }
-        }
-        return null;
-    }
-
     /**
      * Returns the field this Comparator compares by.
      *

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());
+    }
+}