Merge "Increase the stability of sorting in the Model."

2 files changed, 61 insertions, 3 deletions

diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java
index f2bade588f23..cf21d1547b02 100644
--- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java
+++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/Model.java
@@ -187,7 +187,7 @@ public class Model implements SiblingProvider {
                     }
                     break;
                 case SORT_ORDER_LAST_MODIFIED:
-                    longValues[pos] = getCursorLong(mCursor, Document.COLUMN_LAST_MODIFIED);
+                    longValues[pos] = getLastModified(mCursor);
                     stringValues[pos] = getCursorString(mCursor, Document.COLUMN_MIME_TYPE);
                     break;
                 case SORT_ORDER_SIZE:
@@ -309,11 +309,19 @@ public class Model implements SiblingProvider {
                 } else {
                     final long lhs = pivotValue;
                     final long rhs = sortKey[mid];
-                    // Sort in descending numerical order. This matches legacy behaviour, which yields
-                    // largest or most recent items on top.
+                    // Sort in descending numerical order. This matches legacy behaviour, which
+                    // yields largest or most recent items on top.
                     compare = -Long.compare(lhs, rhs);
                 }
 
+                // If numerical comparison yields a tie, use document ID as a tie breaker.  This
+                // will yield stable results even if incoming items are continually shuffling and
+                // have identical numerical sort keys.  One common example of this scenario is seen
+                // when sorting a set of active downloads by mod time.
+                if (compare == 0) {
+                    compare = pivotId.compareTo(ids.get(mid));
+                }
+
                 if (compare < 0) {
                     right = mid;
                 } else {
@@ -350,6 +358,16 @@ public class Model implements SiblingProvider {
         }
     }
 
+    /**
+     * @return Timestamp for the given document. Some docs (e.g. active downloads) have a null
+     * timestamp - these will be replaced with MAX_LONG so that such files get sorted to the top
+     * when sorting by date.
+     */
+    long getLastModified(Cursor cursor) {
+        long l = getCursorLong(mCursor, Document.COLUMN_LAST_MODIFIED);
+        return (l == -1) ? Long.MAX_VALUE : l;
+    }
+
     @Nullable Cursor getItem(String modelId) {
         Integer pos = mPositions.get(modelId);
         if (pos != null) {
diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java
index bed7c9c9a253..a5f065638c96 100644
--- a/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java
+++ b/packages/DocumentsUI/tests/src/com/android/documentsui/dirlist/ModelTest.java
@@ -34,8 +34,10 @@ import com.android.documentsui.model.DocumentInfo;
 
 import java.util.ArrayList;
 import java.util.BitSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Random;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 
 @SmallTest
@@ -50,6 +52,7 @@ public class ModelTest extends AndroidTestCase {
         Document.COLUMN_FLAGS,
         Document.COLUMN_DISPLAY_NAME,
         Document.COLUMN_SIZE,
+        Document.COLUMN_LAST_MODIFIED,
         Document.COLUMN_MIME_TYPE
     };
 
@@ -263,6 +266,43 @@ public class ModelTest extends AndroidTestCase {
         assertEquals(ITEM_COUNT, seen.cardinality());
     }
 
+    public void testSort_time() {
+        final int DL_COUNT = 3;
+        MatrixCursor c = new MatrixCursor(COLUMNS);
+        Set<String> currentDownloads = new HashSet<>();
+
+        // Add some files
+        for (int i = 0; i < ITEM_COUNT; i++) {
+            MatrixCursor.RowBuilder row = c.newRow();
+            row.add(RootCursorWrapper.COLUMN_AUTHORITY, AUTHORITY);
+            row.add(Document.COLUMN_DOCUMENT_ID, Integer.toString(i));
+            row.add(Document.COLUMN_LAST_MODIFIED, System.currentTimeMillis());
+        }
+        // Add some current downloads (no timestamp)
+        for (int i = ITEM_COUNT; i < ITEM_COUNT + DL_COUNT; i++) {
+            MatrixCursor.RowBuilder row = c.newRow();
+            String id = Integer.toString(i);
+            row.add(RootCursorWrapper.COLUMN_AUTHORITY, AUTHORITY);
+            row.add(Document.COLUMN_DOCUMENT_ID, id);
+            currentDownloads.add(Model.createModelId(AUTHORITY, id));
+        }
+
+        DirectoryResult r = new DirectoryResult();
+        r.cursor = c;
+        r.sortOrder = State.SORT_ORDER_LAST_MODIFIED;
+        model.update(r);
+
+        List<String> ids = model.getModelIds();
+
+        // Check that all items were accounted for
+        assertEquals(ITEM_COUNT + DL_COUNT, ids.size());
+
+        // Check that active downloads are sorted to the top.
+        for (int i = 0; i < DL_COUNT; i++) {
+            assertTrue(currentDownloads.contains(ids.get(i)));
+        }
+    }
+
     // Tests that Model.delete works correctly.
     public void testDelete() throws Exception {
         // Simulate deleting 2 files.