fix upload list comparator to make it respect the comparator contract

Signed-off-by: Daniele Fognini <dfogni@gmail.com>
nextcloud · Jul 20, 2019 · 6541907 · 6541907
1 parent 5b0a53d
commit 6541907
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 41 deletions.
diff --git a/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java b/src/main/java/com/owncloud/android/ui/adapter/UploadListAdapter.java
@@ -681,6 +681,59 @@ enum Type {
         CURRENT, FINISHED, FAILED
     }
 
+    static Comparator<OCUpload> comparator = new Comparator<OCUpload>() {
+        @Override
+        public int compare(OCUpload upload1, OCUpload upload2) {
+            if (upload1 == null && upload2 == null) {
+                return 0;
+            }
+            if (upload1 == null) {
+                return -1;
+            }
+            if (upload2 == null) {
+                return 1;
+            }
+
+            int compareUploadStatus = compareUploadStatus(upload1, upload2);
+            if (compareUploadStatus != 0) {
+                return compareUploadStatus;
+            }
+
+            int compareUploadingNow = compareUploadingNow(upload1, upload2);
+            if (compareUploadingNow != 0) {
+                return compareUploadingNow;
+            }
+
+            int compareUpdateTime = compareUpdateTime(upload1, upload2);
+            if (compareUpdateTime != 0) {
+                return compareUpdateTime;
+            }
+
+            int compareUploadId = compareUploadId(upload1, upload2);
+            if (compareUploadId != 0) {
+                return compareUploadId;
+            }
+
+            return 0;
+        }
+
+        private int compareUploadStatus(OCUpload upload1, OCUpload upload2) {
+            return upload1.getFixedUploadStatus().compareTo(upload2.getFixedUploadStatus());
+        }
+
+        private int compareUploadingNow(OCUpload upload1, OCUpload upload2) {
+            return -Boolean.compare(upload1.isFixedUploadingNow(), upload2.isFixedUploadingNow());
+        }
+
+        private int compareUploadId(OCUpload upload1, OCUpload upload2) {
+            return Long.compare(upload1.getFixedUploadId(), upload2.getFixedUploadId());
+        }
+
+        private int compareUpdateTime(OCUpload upload1, OCUpload upload2) {
+            return Long.compare(upload2.getFixedUploadEndTimeStamp(), upload1.getFixedUploadEndTimeStamp());
+        }
+    };
+
     abstract class UploadGroup implements Refresh {
         private Type type;
         private OCUpload[] items;
@@ -722,46 +775,5 @@ void fixAndSortItems(OCUpload... array) {
         private int getGroupItemCount() {
             return items == null ? 0 : items.length;
         }
-
-        Comparator<OCUpload> comparator = new Comparator<OCUpload>() {
-            @Override
-            public int compare(OCUpload upload1, OCUpload upload2) {
-                if (upload1 == null && upload2 == null) {
-                    return 0;
-                }
-                if (upload1 == null) {
-                    return -1;
-                }
-                if (upload2 == null) {
-                    return 1;
-                }
-                if (UploadStatus.UPLOAD_IN_PROGRESS == upload1.getFixedUploadStatus()) {
-                    if (UploadStatus.UPLOAD_IN_PROGRESS != upload2.getFixedUploadStatus()) {
-                        return -1;
-                    }
-                    // both are in progress
-                    if (upload1.isFixedUploadingNow()) {
-                        return -1;
-                    } else if (upload2.isFixedUploadingNow()) {
-                        return 1;
-                    }
-                } else if (upload2.getFixedUploadStatus() == UploadStatus.UPLOAD_IN_PROGRESS) {
-                    return 1;
-                }
-                if (upload1.getFixedUploadEndTimeStamp() == 0 || upload2.getFixedUploadEndTimeStamp() == 0) {
-                    return compareUploadId(upload1, upload2);
-                } else {
-                    return compareUpdateTime(upload1, upload2);
-                }
-            }
-
-            private int compareUploadId(OCUpload upload1, OCUpload upload2) {
-                return Long.compare(upload1.getFixedUploadId(), upload2.getFixedUploadId());
-            }
-
-            private int compareUpdateTime(OCUpload upload1, OCUpload upload2) {
-                return Long.compare(upload2.getFixedUploadEndTimeStamp(), upload1.getFixedUploadEndTimeStamp());
-            }
-        };
     }
 }
diff --git a/src/test/java/com/owncloud/android/ui/adapter/UploadListAdapterTest.java b/src/test/java/com/owncloud/android/ui/adapter/UploadListAdapterTest.java
@@ -0,0 +1,38 @@
+package com.owncloud.android.ui.adapter;
+
+import com.owncloud.android.datamodel.UploadsStorageManager.UploadStatus;
+import com.owncloud.android.db.OCUpload;
+
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Comparator;
+
+import static org.mockito.Mockito.mock;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.when;
+
+public class UploadListAdapterTest {
+    @Test
+    public void testUploadListComparator() {
+        Comparator<OCUpload> comparator = UploadListAdapter.comparator;
+
+        OCUpload upload1 = mock(OCUpload.class, "upload1");
+        OCUpload upload2 = mock(OCUpload.class, "upload2");
+        OCUpload upload3 = mock(OCUpload.class, "upload3");
+
+        when(upload1.getFixedUploadStatus()).thenReturn(UploadStatus.UPLOAD_FAILED);
+        when(upload2.getFixedUploadStatus()).thenReturn(UploadStatus.UPLOAD_IN_PROGRESS);
+        when(upload3.getFixedUploadStatus()).thenReturn(UploadStatus.UPLOAD_IN_PROGRESS);
+
+        when(upload3.isFixedUploadingNow()).thenReturn(true);
+        when(upload2.isFixedUploadingNow()).thenReturn(false);
+
+        OCUpload[] uploads = new OCUpload[]{upload1, upload2, upload3};
+        Arrays.sort(uploads, comparator);
+
+        assertArrayEquals(new OCUpload[]{upload3, upload2, upload1}, uploads);
+    }
+
+}