feeditem sort - handle boundary conditions

2 files changed, 93 insertions, 26 deletions

diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/FeedItemPermutors.java b/core/src/main/java/de/danoeh/antennapod/core/util/FeedItemPermutors.java
index 2d1d6e658..9bf9b9db0 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/util/FeedItemPermutors.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/util/FeedItemPermutors.java
@@ -1,16 +1,17 @@
 package de.danoeh.antennapod.core.util;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import de.danoeh.antennapod.core.feed.FeedItem;
-import de.danoeh.antennapod.core.feed.FeedMedia;
 
 /**
  * Provides method for sorting the a list of {@link FeedItem} according to rules.
@@ -30,45 +31,28 @@ public class FeedItemPermutors {
 
         switch (sortOrder) {
             case EPISODE_TITLE_A_Z:
-                comparator = (f1, f2) -> f1.getTitle().compareTo(f2.getTitle());
+                comparator = (f1, f2) -> itemTitle(f1).compareTo(itemTitle(f2));
                 break;
             case EPISODE_TITLE_Z_A:
-                comparator = (f1, f2) -> f2.getTitle().compareTo(f1.getTitle());
+                comparator = (f1, f2) -> itemTitle(f2).compareTo(itemTitle(f1));
                 break;
             case DATE_OLD_NEW:
-                comparator = (f1, f2) -> f1.getPubDate().compareTo(f2.getPubDate());
+                comparator = (f1, f2) -> pubDate(f1).compareTo(pubDate(f2));
                 break;
             case DATE_NEW_OLD:
-                comparator = (f1, f2) -> f2.getPubDate().compareTo(f1.getPubDate());
+                comparator = (f1, f2) -> pubDate(f2).compareTo(pubDate(f1));
                 break;
             case DURATION_SHORT_LONG:
-                comparator = (f1, f2) -> {
-                    FeedMedia f1Media = f1.getMedia();
-                    FeedMedia f2Media = f2.getMedia();
-                    int duration1 = f1Media != null ? f1Media.getDuration() : -1;
-                    int duration2 = f2Media != null ? f2Media.getDuration() : -1;
-
-                    if (duration1 == -1 || duration2 == -1)
-                        return duration2 - duration1;
-                    else
-                        return duration1 - duration2;
-                };
+                comparator = (f1, f2) -> Integer.compare(duration(f1), duration(f2));
                 break;
             case DURATION_LONG_SHORT:
-                comparator = (f1, f2) -> {
-                    FeedMedia f1Media = f1.getMedia();
-                    FeedMedia f2Media = f2.getMedia();
-                    int duration1 = f1Media != null ? f1Media.getDuration() : -1;
-                    int duration2 = f2Media != null ? f2Media.getDuration() : -1;
-
-                    return -1 * (duration1 - duration2);
-                };
+                comparator = (f1, f2) -> Integer.compare(duration(f2), duration(f1));
                 break;
             case FEED_TITLE_A_Z:
-                comparator = (f1, f2) -> f1.getFeed().getTitle().compareTo(f2.getFeed().getTitle());
+                comparator = (f1, f2) -> feedTitle(f1).compareTo(feedTitle(f2));
                 break;
             case FEED_TITLE_Z_A:
-                comparator = (f1, f2) -> f2.getFeed().getTitle().compareTo(f1.getFeed().getTitle());
+                comparator = (f1, f2) -> feedTitle(f2).compareTo(feedTitle(f1));
                 break;
             case RANDOM:
                 permutor = Collections::shuffle;
@@ -88,6 +72,31 @@ public class FeedItemPermutors {
         return permutor;
     }
 
+    // Null-safe accessors
+
+    @NonNull
+    private static Date pubDate(@Nullable FeedItem item) {
+        return (item != null && item.getPubDate() != null) ?
+                item.getPubDate() : new Date(0);
+    }
+
+    @NonNull
+    private static String itemTitle(@Nullable FeedItem item) {
+        return (item != null && item.getTitle() != null) ?
+                item.getTitle() : "";
+    }
+
+    private static int duration(@Nullable FeedItem item) {
+        return (item != null && item.getMedia() != null) ?
+                item.getMedia().getDuration() : 0;
+    }
+
+    @NonNull
+    private static String feedTitle(@Nullable FeedItem item) {
+        return (item != null && item.getFeed() != null && item.getFeed().getTitle() != null) ?
+                item.getFeed().getTitle() : "";
+    }
+
     /**
      * Implements a reordering by pubdate that avoids consecutive episodes from the same feed in
      * the queue.
diff --git a/core/src/test/java/de/danoeh/antennapod/core/util/FeedItemPermutorsTest.java b/core/src/test/java/de/danoeh/antennapod/core/util/FeedItemPermutorsTest.java
index 82b3a794e..f66c77c3d 100644
--- a/core/src/test/java/de/danoeh/antennapod/core/util/FeedItemPermutorsTest.java
+++ b/core/src/test/java/de/danoeh/antennapod/core/util/FeedItemPermutorsTest.java
@@ -10,6 +10,7 @@ import de.danoeh.antennapod.core.feed.Feed;
 import de.danoeh.antennapod.core.feed.FeedItem;
 import de.danoeh.antennapod.core.feed.FeedMedia;
 
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -18,6 +19,14 @@ import static org.junit.Assert.assertTrue;
 public class FeedItemPermutorsTest {
 
     @Test
+    public void testEnsureNonNullPermutors() {
+        for(SortOrder sortOrder : SortOrder.values()) {
+            assertNotNull("The permutor for SortOrder " + sortOrder + " is unexpectedly null",
+                    FeedItemPermutors.getPermutor(sortOrder));
+        }
+    }
+
+    @Test
     public void testPermutorForRule_EPISODE_TITLE_ASC() {
         Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.EPISODE_TITLE_A_Z);
 
@@ -28,6 +37,19 @@ public class FeedItemPermutorsTest {
     }
 
     @Test
+    public void testPermutorForRule_EPISODE_TITLE_ASC_NullTitle() {
+        Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.EPISODE_TITLE_A_Z);
+
+        List<FeedItem> itemList = getTestList();
+        itemList.get(2) // itemId 2
+                .setTitle(null);
+        assertTrue(checkIdOrder(itemList, 1, 3, 2)); // before sorting
+        permutor.reorder(itemList);
+        assertTrue(checkIdOrder(itemList, 2, 1, 3)); // after sorting
+    }
+
+
+    @Test
     public void testPermutorForRule_EPISODE_TITLE_DESC() {
         Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.EPISODE_TITLE_Z_A);
 
@@ -48,6 +70,18 @@ public class FeedItemPermutorsTest {
     }
 
     @Test
+    public void testPermutorForRule_DATE_ASC_NulPubDatel() {
+        Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.DATE_OLD_NEW);
+
+        List<FeedItem> itemList = getTestList();
+        itemList.get(2) // itemId 2
+                .setPubDate(null);
+        assertTrue(checkIdOrder(itemList, 1, 3, 2)); // before sorting
+        permutor.reorder(itemList);
+        assertTrue(checkIdOrder(itemList, 2, 1, 3)); // after sorting
+    }
+
+    @Test
     public void testPermutorForRule_DATE_DESC() {
         Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.DATE_NEW_OLD);
 
@@ -78,6 +112,18 @@ public class FeedItemPermutorsTest {
     }
 
     @Test
+    public void testPermutorForRule_DURATION_DESC_NullMedia() {
+        Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.DURATION_LONG_SHORT);
+
+        List<FeedItem> itemList = getTestList();
+        itemList.get(1) // itemId 3
+                .setMedia(null);
+        assertTrue(checkIdOrder(itemList, 1, 3, 2)); // before sorting
+        permutor.reorder(itemList);
+        assertTrue(checkIdOrder(itemList, 2, 1, 3)); // after sorting
+    }
+
+    @Test
     public void testPermutorForRule_FEED_TITLE_ASC() {
         Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.FEED_TITLE_A_Z);
 
@@ -97,6 +143,18 @@ public class FeedItemPermutorsTest {
         assertTrue(checkIdOrder(itemList, 3, 2, 1)); // after sorting
     }
 
+    @Test
+    public void testPermutorForRule_FEED_TITLE_DESC_NullTitle() {
+        Permutor<FeedItem> permutor = FeedItemPermutors.getPermutor(SortOrder.FEED_TITLE_Z_A);
+
+        List<FeedItem> itemList = getTestList();
+        itemList.get(1) // itemId 3
+            .getFeed().setTitle(null);
+        assertTrue(checkIdOrder(itemList, 1, 3, 2)); // before sorting
+        permutor.reorder(itemList);
+        assertTrue(checkIdOrder(itemList, 2, 1, 3)); // after sorting
+    }
+
     /**
      * Generates a list with test data.
      */