Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e433834d authored by Ben Kwa's avatar Ben Kwa
Browse files

Increase the stability of sorting in the Model.

This is necessary to prevent UI thrash (items continually shuffling
around) when the underlying Provider doesn't return its contents in a
stable order from one load event to another (DownloadStorageProvider is
an example).

BUG=26417297

Change-Id: Ie99e56b610f5d01d5318be07c0379b506c828735
parent c625816a
Loading
Loading
Loading
Loading
+21 −3
Original line number Diff line number Diff line
@@ -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) {
+40 −0
Original line number Diff line number Diff line
@@ -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.