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

Commit 9fca541a authored by Daichi Hirono's avatar Daichi Hirono
Browse files

Relax mapping rule to make the mapping logic simple. 

MtpDocumentsProvider remembers the mapping between SAF's ID and MTP's
ID. Sometimes we need to do heuristic to restore the mapping when MTP
device is reconnected.

Previously we do the mapping files that shares the same name more
strictly. For example,

1. Found file name "test.txt". Assign document ID "1".
2. MTP device is disconnected and the MTP ID of "1" is lost.
3. Found two files that have same name "test.txt" in the same directory.

Previously we don't reuse existing document ID "1" for neither of two
"test.txt" because it's not 1-to-1 mapping and we cannot determine which
one should be mapped with existing document ID. It means we need the
complete list of files in a directory to remap IDs. It takes long time
to fetch all file names in a directory when a directory has 100+
files. It's rare that a MTP device has the two files sharing the same
name in the same directory. Also the strict rule makes the mapping code
more complex.

The CL relax the rule of mapping, and it allows to reuse existing
document ID even if it is not 1-to-1 mapping. For the previous example,
it assigns "1" for either of "test.txt".

BUG=27053734
Change-Id: I19406fafc21f13ab94ba99411ce5e7f55ce7f658
(cherry picked from commit acdbc6e7)
parent 8b67c169

packages/MtpDocumentsProvider/src/com/android/mtp/Mapper.java

+1 −145
Original line number Diff line number Diff line
@@ -73,7 +73,6 @@ class Mapper { 
                    extraValuesList,

                    COLUMN_PARENT_DOCUMENT_ID + " IS NULL",

                    EMPTY_ARGS,

                    /* heuristic */ false,

                    COLUMN_DEVICE_ID);

            database.setTransactionSuccessful();

            return changed;
@@ -92,16 +91,13 @@ class Mapper { 
        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        database.beginTransaction();

        try {

            final boolean heuristic;

            final String mapColumn;

            Preconditions.checkState(mMappingMode.containsKey(parentDocumentId));

            switch (mMappingMode.get(parentDocumentId)) {

                case MAP_BY_MTP_IDENTIFIER:

                    heuristic = false;

                    mapColumn = COLUMN_STORAGE_ID;

                    break;

                case MAP_BY_NAME:

                    heuristic = true;

                    mapColumn = Document.COLUMN_DISPLAY_NAME;

                    break;

                default:
@@ -120,7 +116,6 @@ class Mapper { 
                    extraValuesList,

                    COLUMN_PARENT_DOCUMENT_ID + "=?",

                    strings(parentDocumentId),

                    heuristic,

                    mapColumn);



            database.setTransactionSuccessful();
@@ -137,16 +132,13 @@ class Mapper { 
     * @param documents List of document information.

     */

    synchronized void putChildDocuments(int deviceId, String parentId, MtpObjectInfo[] documents) {

        final boolean heuristic;

        final String mapColumn;

        Preconditions.checkState(mMappingMode.containsKey(parentId));

        switch (mMappingMode.get(parentId)) {

            case MAP_BY_MTP_IDENTIFIER:

                heuristic = false;

                mapColumn = COLUMN_OBJECT_HANDLE;

                break;

            case MAP_BY_NAME:

                heuristic = true;

                mapColumn = Document.COLUMN_DISPLAY_NAME;

                break;

            default:
@@ -163,17 +155,13 @@ class Mapper { 
                null,

                COLUMN_PARENT_DOCUMENT_ID + "=?",

                strings(parentId),

                heuristic,

                mapColumn);

    }



    @VisibleForTesting

    void clearMapping() {

        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        database.beginTransaction();

        try {

            mDatabase.deleteDocumentsAndRootsRecursively(

                    COLUMN_ROW_STATE + " = ?", strings(ROW_STATE_PENDING));

            final ContentValues values = new ContentValues();

            values.putNull(COLUMN_OBJECT_HANDLE);

            values.putNull(COLUMN_STORAGE_ID);
@@ -209,11 +197,6 @@ class Mapper { 
        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        database.beginTransaction();

        try {

            // Delete all pending rows.

            mDatabase.deleteDocumentsAndRootsRecursively(

                    selection + " AND " + COLUMN_ROW_STATE + "=?",

                    DatabaseUtils.appendSelectionArgs(args, strings(ROW_STATE_PENDING)));



            // Set all documents as invalidated.

            final ContentValues values = new ContentValues();

            values.put(COLUMN_ROW_STATE, ROW_STATE_INVALIDATED);
@@ -245,7 +228,6 @@ class Mapper { 
     * @param rootExtraValuesList Values for root extra to be stored in the database.

     * @param selection SQL where closure to select rows that shares the same parent.

     * @param args Argument for selection SQL.

     * @param heuristic Whether the mapping mode is heuristic.

     * @return Whether the method adds new rows.

     */

    private boolean putDocuments(
@@ -253,7 +235,6 @@ class Mapper { 
            @Nullable ContentValues[] rootExtraValuesList,

            String selection,

            String[] args,

            boolean heuristic,

            String mappingKey) {

        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        boolean added = false;
@@ -285,7 +266,7 @@ class Mapper { 
                    if (candidateCursor.getCount() == 0) {

                        rowId = database.insert(TABLE_DOCUMENTS, null, values);

                        added = true;

                    } else if (!heuristic) {

                    } else {

                        candidateCursor.moveToNext();

                        rowId = candidateCursor.getLong(0);

                        database.update(
@@ -293,9 +274,6 @@ class Mapper { 
                                values,

                                SELECTION_DOCUMENT_ID,

                                strings(rowId));

                    } else {

                        values.put(COLUMN_ROW_STATE, ROW_STATE_PENDING);

                        rowId = database.insertOrThrow(TABLE_DOCUMENTS, null, values);

                    }

                    // Document ID is a primary integer key of the table. So the returned row

                    // IDs should be same with the document ID.
@@ -334,84 +312,10 @@ class Mapper { 
            selection = COLUMN_PARENT_DOCUMENT_ID + " IS NULL";

            args = EMPTY_ARGS;

        }

        final String groupKey;

        switch (mMappingMode.get(parentId)) {

            case MAP_BY_MTP_IDENTIFIER:

                groupKey = parentId != null ? COLUMN_OBJECT_HANDLE : COLUMN_STORAGE_ID;

                break;

            case MAP_BY_NAME:

                groupKey = Document.COLUMN_DISPLAY_NAME;

                break;

            default:

                throw new Error("Unexpected mapping state.");

        }

        mMappingMode.remove(parentId);

        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        database.beginTransaction();

        try {

            // Get 1-to-1 mapping of invalidated document and pending document.

            final String invalidatedIdQuery = createStateFilter(

                    ROW_STATE_INVALIDATED, Document.COLUMN_DOCUMENT_ID);

            final String pendingIdQuery = createStateFilter(

                    ROW_STATE_PENDING, Document.COLUMN_DOCUMENT_ID);

            // SQL should be like:

            // SELECT group_concat(CASE WHEN raw_state = 1 THEN document_id ELSE NULL END),

            //        group_concat(CASE WHEN raw_state = 2 THEN document_id ELSE NULL END)

            // WHERE device_id = ? AND parent_document_id IS NULL

            // GROUP BY display_name

            // HAVING count(CASE WHEN raw_state = 1 THEN document_id ELSE NULL END) = 1 AND

            //        count(CASE WHEN raw_state = 2 THEN document_id ELSE NULL END) = 1

            final Cursor mergingCursor = database.query(

                    TABLE_DOCUMENTS,

                    new String[] {

                            "group_concat(" + invalidatedIdQuery + ")",

                            "group_concat(" + pendingIdQuery + ")"

                    },

                    selection,

                    args,

                    groupKey,

                    "count(" + invalidatedIdQuery + ") = 1 AND count(" + pendingIdQuery + ") = 1",

                    null);



            final ContentValues values = new ContentValues();

            while (mergingCursor.moveToNext()) {

                final String invalidatedId = mergingCursor.getString(0);

                final String pendingId = mergingCursor.getString(1);



                // Obtain the new values including the latest object handle from mapping row.

                getFirstRow(

                        TABLE_DOCUMENTS,

                        SELECTION_DOCUMENT_ID,

                        new String[] { pendingId },

                        values);

                values.remove(Document.COLUMN_DOCUMENT_ID);

                values.put(COLUMN_ROW_STATE, ROW_STATE_VALID);

                database.update(

                        TABLE_DOCUMENTS,

                        values,

                        SELECTION_DOCUMENT_ID,

                        new String[] { invalidatedId });



                getFirstRow(

                        TABLE_ROOT_EXTRA,

                        SELECTION_ROOT_ID,

                        new String[] { pendingId },

                        values);

                if (values.size() > 0) {

                    values.remove(Root.COLUMN_ROOT_ID);

                    database.update(

                            TABLE_ROOT_EXTRA,

                            values,

                            SELECTION_ROOT_ID,

                            new String[] { invalidatedId });

                }



                // Delete 'pending' row.

                mDatabase.deleteDocumentsAndRootsRecursively(

                        SELECTION_DOCUMENT_ID, new String[] { pendingId });

            }

            mergingCursor.close();



            boolean changed = false;



            // Delete all invalidated rows that cannot be mapped.
@@ -421,58 +325,10 @@ class Mapper { 
                changed = true;

            }



            // The database cannot find old document ID for the pending rows.

            // Turn the all pending rows into valid state, which means the rows become to be

            // valid with new document ID.

            values.clear();

            values.put(COLUMN_ROW_STATE, ROW_STATE_VALID);

            if (database.update(

                    TABLE_DOCUMENTS,

                    values,

                    COLUMN_ROW_STATE + " = ? AND " + selection,

                    DatabaseUtils.appendSelectionArgs(strings(ROW_STATE_PENDING), args)) != 0) {

                changed = true;

            }

            database.setTransactionSuccessful();

            return changed;

        } finally {

            database.endTransaction();

        }

    }



    /**

     * Obtains values of the first row for the query.

     * @param values ContentValues that the values are stored to.

     * @param table Target table.

     * @param selection Query to select rows.

     * @param args Argument for query.

     */

    private void getFirstRow(String table, String selection, String[] args, ContentValues values) {

        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        values.clear();

        final Cursor cursor = database.query(table, null, selection, args, null, null, null, "1");

        try {

            if (cursor.getCount() == 0) {

                return;

            }

            cursor.moveToNext();

            DatabaseUtils.cursorRowToContentValues(cursor, values);

        } finally {

            cursor.close();

        }

    }



    /**

     * Gets SQL expression that represents the given value or NULL depends on the row state.

     * You must pass static constants to this methods otherwise you may be suffered from SQL

     * injections.

     * @param state Expected row state.

     * @param a SQL value.

     * @return Expression that represents a if the row state is expected one, and represents NULL

     *     otherwise.

     */

    private static String createStateFilter(int state, String a) {

        return "CASE WHEN " + COLUMN_ROW_STATE + " = " + Integer.toString(state) +

                " THEN " + a + " ELSE NULL END";

    }

}

packages/MtpDocumentsProvider/src/com/android/mtp/MtpDatabaseConstants.java

+0 −8
Original line number Diff line number Diff line
@@ -77,14 +77,6 @@ class MtpDatabaseConstants { 
     */

    static final int ROW_STATE_INVALIDATED = 1;



    /**

     * The state represents the raw has a valid object handle but it may be going to be mapped with

     * another rows invalidated. After fetching all documents under the parent, the database tries

     * to map the pending documents and the invalidated documents in order to keep old document ID

     * alive.

     */

    static final int ROW_STATE_PENDING = 2;



    /**

     * Mapping mode that uses MTP identifier to find corresponding rows.

     */

packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDatabaseTest.java

+9 −8
Original line number Diff line number Diff line
@@ -310,14 +310,14 @@ public class MtpDatabaseTest extends AndroidTestCase { 
            assertEquals(3, cursor.getCount());

            cursor.moveToNext();

            assertEquals(1, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertTrue(isNull(cursor, COLUMN_STORAGE_ID));

            assertEquals(200, getInt(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage A", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.moveToNext();

            assertEquals(2, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertTrue(isNull(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage B", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.moveToNext();

            assertEquals(4, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(3, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(202, getInt(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage C", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.close();
@@ -333,7 +333,7 @@ public class MtpDatabaseTest extends AndroidTestCase { 
            assertEquals(200, getInt(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage A", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.moveToNext();

            assertEquals(4, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(3, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(202, getInt(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage C", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.close();
@@ -387,7 +387,7 @@ public class MtpDatabaseTest extends AndroidTestCase { 
            assertEquals(4, cursor.getCount());



            cursor.moveToPosition(3);

            assertEquals(5, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(4, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(203, getInt(cursor, COLUMN_OBJECT_HANDLE));

            assertEquals("video.mp4", getString(cursor, COLUMN_DISPLAY_NAME));
@@ -406,7 +406,7 @@ public class MtpDatabaseTest extends AndroidTestCase { 
            assertEquals("note.txt", getString(cursor, COLUMN_DISPLAY_NAME));



            cursor.moveToNext();

            assertEquals(5, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(4, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(203, getInt(cursor, COLUMN_OBJECT_HANDLE));

            assertEquals("video.mp4", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.close();
@@ -544,7 +544,7 @@ public class MtpDatabaseTest extends AndroidTestCase { 
            assertEquals(1, cursor.getCount());

            cursor.moveToNext();

            assertEquals(2, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertTrue(isNull(cursor, COLUMN_OBJECT_HANDLE));

            assertEquals(201, getInt(cursor, COLUMN_OBJECT_HANDLE));

            cursor.close();

        }

    }
@@ -626,11 +626,12 @@ public class MtpDatabaseTest extends AndroidTestCase { 
            final Cursor cursor = mDatabase.queryRootDocuments(columns);

            assertEquals(2, cursor.getCount());

            cursor.moveToNext();

            assertEquals(2, getInt(cursor, COLUMN_DOCUMENT_ID));

            // One reuses exisitng document ID 1.

            assertEquals(1, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(200, getInt(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.moveToNext();

            assertEquals(3, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(2, getInt(cursor, COLUMN_DOCUMENT_ID));

            assertEquals(201, getInt(cursor, COLUMN_STORAGE_ID));

            assertEquals("Storage", getString(cursor, COLUMN_DISPLAY_NAME));

            cursor.close();