Update STANDARD_MIMETYPE_EXTENSION in picker db during idle maintenance

Refactored DbWriteOperation and its subclasses in PickerDbFacade to support media updates and to segregate album operations. During idle maintenance in the MediaProvider, calculate special format for files and invoke PickerDbFacade to update the STANDARD_MIMETYPE_EXTENSION column in the picker db.

Test: atest PickerDbFacadeTest, IdleServiceTest
Bug: 236620024
Change-Id: Ia86fcba6c7f65dfdb68c68d6a4149f02ba0e9d26

4 files changed, 185 insertions, 66 deletions

diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 60e01a507..5e1009927 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -44,7 +44,6 @@ import static android.provider.MediaStore.QUERY_ARG_MATCH_PENDING;
 import static android.provider.MediaStore.QUERY_ARG_MATCH_TRASHED;
 import static android.provider.MediaStore.QUERY_ARG_REDACTED_URI;
 import static android.provider.MediaStore.QUERY_ARG_RELATED_URI;
-import static android.provider.MediaStore.VOLUME_EXTERNAL;
 import static android.provider.MediaStore.getVolumeName;
 import static android.system.OsConstants.F_GETFL;
 
@@ -1313,9 +1312,9 @@ public class MediaProvider extends ContentProvider {
         // Cleaning media files for users that have been removed
         cleanMediaFilesForRemovedUser(signal);
 
-        // Populate _SPECIAL_FORMAT column for files which have column value as NULL
-        // TODO(b/236620024): Do not update generation_modified for special_format value update
-        // detectSpecialFormat(signal);
+        // Calculate standard_mime_type_extension column for files which have SPECIAL_FORMAT column
+        // value as NULL, and update the same in the picker db
+        detectSpecialFormat(signal);
 
         final long durationMillis = (SystemClock.elapsedRealtime() - startTime);
         Metrics.logIdleMaintenance(MediaStore.VOLUME_EXTERNAL, itemCount,
@@ -1428,9 +1427,14 @@ public class MediaProvider extends ContentProvider {
     private void updateSpecialFormatColumn(SQLiteDatabase db, @NonNull CancellationSignal signal) {
         // This is to ensure we only do a bounded iteration over the rows as updates can fail, and
         // we don't want to keep running the query/update indefinitely.
-        final int totalRowsToUpdate = getPendingSpecialFormatRowsCount(db,signal);
-        for (int i = 0 ; i < totalRowsToUpdate ; i += IDLE_MAINTENANCE_ROWS_LIMIT) {
-            updateSpecialFormatForLimitedRows(db, signal);
+        final int totalRowsToUpdate = getPendingSpecialFormatRowsCount(db, signal);
+        for (int i = 0; i < totalRowsToUpdate; i += IDLE_MAINTENANCE_ROWS_LIMIT) {
+            try (PickerDbFacade.UpdateMediaOperation operation =
+                         mPickerDbFacade.beginUpdateMediaOperation(
+                                 PickerSyncController.LOCAL_PICKER_PROVIDER_AUTHORITY)) {
+                updateSpecialFormatForLimitedRows(db, signal, operation);
+                operation.setSuccess();
+            }
         }
     }
 
@@ -1444,14 +1448,12 @@ public class MediaProvider extends ContentProvider {
         }
     }
 
-    private void updateSpecialFormatForLimitedRows(SQLiteDatabase db,
-            @NonNull CancellationSignal signal) {
-        final SQLiteQueryBuilder qbForUpdate = getQueryBuilder(TYPE_UPDATE, FILES,
-                Files.getContentUri(VOLUME_EXTERNAL), Bundle.EMPTY, null);
+    private void updateSpecialFormatForLimitedRows(SQLiteDatabase externalDb,
+            @NonNull CancellationSignal signal, PickerDbFacade.UpdateMediaOperation operation) {
         // Accumulate all the new SPECIAL_FORMAT updates with their ids
         ArrayMap<Long, Integer> newSpecialFormatValues = new ArrayMap<>();
         final String limit = String.valueOf(IDLE_MAINTENANCE_ROWS_LIMIT);
-        try (Cursor c = queryForPendingSpecialFormatColumns(db, limit, signal)) {
+        try (Cursor c = queryForPendingSpecialFormatColumns(externalDb, limit, signal)) {
             while (c.moveToNext() && !signal.isCanceled()) {
                 final long id = c.getLong(0);
                 final String path = c.getString(1);
@@ -1459,25 +1461,35 @@ public class MediaProvider extends ContentProvider {
             }
         }
 
-        // Now, update all the new SPECIAL_FORMAT values.
-        final ContentValues values = new ContentValues();
+        // Now, update all the new SPECIAL_FORMAT values in both external db and picker db.
+        final ContentValues pickerDbValues = new ContentValues();
+        final ContentValues externalDbValues = new ContentValues();
         int count = 0;
-        for (long id: newSpecialFormatValues.keySet()) {
+        for (long id : newSpecialFormatValues.keySet()) {
             if (signal.isCanceled()) {
                 return;
             }
 
-            values.clear();
-            values.put(_SPECIAL_FORMAT, newSpecialFormatValues.get(id));
-            final String selection = MediaColumns._ID + "=?";
-            final String[] selectionArgs = new String[]{String.valueOf(id)};
-            if (qbForUpdate.update(db, values, selection, selectionArgs) == 1) {
+            int specialFormat = newSpecialFormatValues.get(id);
+
+            pickerDbValues.clear();
+            pickerDbValues.put(PickerDbFacade.KEY_STANDARD_MIME_TYPE_EXTENSION, specialFormat);
+            boolean pickerDbWriteSuccess = operation.execute(String.valueOf(id), pickerDbValues);
+
+            externalDbValues.clear();
+            externalDbValues.put(_SPECIAL_FORMAT, specialFormat);
+            final String externalDbSelection = MediaColumns._ID + "=?";
+            final String[] externalDbSelectionArgs = new String[]{String.valueOf(id)};
+            boolean externalDbWriteSuccess =
+                    externalDb.update("files", externalDbValues, externalDbSelection,
+                            externalDbSelectionArgs)
+                            == 1;
+
+            if (pickerDbWriteSuccess && externalDbWriteSuccess) {
                 count++;
-            } else {
-                Log.e(TAG, "Unable to update _SPECIAL_FORMAT for id = " + id);
             }
         }
-        Log.d(TAG, "Updated _SPECIAL_FORMAT for " + count + " items");
+        Log.d(TAG, "Updated standard_mime_type_extension for " + count + " items");
     }
 
     private int getSpecialFormatValue(String path) {
diff --git a/src/com/android/providers/media/photopicker/data/PickerDbFacade.java b/src/com/android/providers/media/photopicker/data/PickerDbFacade.java
index 1a0dda2c1..0f8953ef2 100644
--- a/src/com/android/providers/media/photopicker/data/PickerDbFacade.java
+++ b/src/com/android/providers/media/photopicker/data/PickerDbFacade.java
@@ -37,8 +37,6 @@ import android.database.sqlite.SQLiteConstraintException;
 import android.database.sqlite.SQLiteDatabase;
 import android.database.sqlite.SQLiteQueryBuilder;
 import android.net.Uri;
-import android.os.SystemProperties;
-import android.provider.DeviceConfig;
 import android.provider.CloudMediaProviderContract;
 import android.provider.MediaStore;
 import android.text.TextUtils;
@@ -115,7 +113,6 @@ public class PickerDbFacade {
     public static final String KEY_DURATION_MS = "duration_ms";
     @VisibleForTesting
     public static final String KEY_MIME_TYPE = "mime_type";
-    @VisibleForTesting
     public static final String KEY_STANDARD_MIME_TYPE_EXTENSION = "standard_mime_type_extension";
     @VisibleForTesting
     public static final String KEY_IS_FAVORITE = "is_favorite";
@@ -240,6 +237,10 @@ public class PickerDbFacade {
      * Returns {@link DbWriteOperation} to clear album media for a given albumId from the picker
      * db.
      *
+     * <p>The {@link DbWriteOperation} clears local or cloud album based on {@code authority} and
+     * {@code albumId}. If {@code albumId} is null, it clears all local or cloud albums based on
+     * {@code authority}.
+     *
      * @param authority to determine whether local or cloud media should be cleared
      */
     public DbWriteOperation beginResetAlbumMediaOperation(String authority, String albumId) {
@@ -247,37 +248,38 @@ public class PickerDbFacade {
     }
 
     /**
+     * Returns {@link UpdateMediaOperation} to update media belonging to {@code authority} in the
+     * picker db.
+     *
+     * @param authority to determine whether local or cloud media should be updated
+     */
+    public UpdateMediaOperation beginUpdateMediaOperation(String authority) {
+        return new UpdateMediaOperation(mDatabase, isLocal(authority));
+    }
+
+    /**
      * Represents an atomic write operation to the picker database.
      *
      * <p>This class is not thread-safe and is meant to be used within a single thread only.
      */
-    public static abstract class DbWriteOperation implements AutoCloseable {
+    public abstract static class DbWriteOperation implements AutoCloseable {
 
         private final SQLiteDatabase mDatabase;
         private final boolean mIsLocal;
-        private final String mAlbumId;
 
         private boolean mIsSuccess = false;
 
-        // Needed for Album Media Write operations.
         private DbWriteOperation(SQLiteDatabase database, boolean isLocal) {
-            this(database, isLocal, "");
-        }
-
-        // Needed for Album Media Write operations.
-        private DbWriteOperation(SQLiteDatabase database, boolean isLocal, String albumId) {
             mDatabase = database;
             mIsLocal = isLocal;
-            mAlbumId = albumId;
             mDatabase.beginTransaction();
         }
 
-        /*
-         * Execute the write operation.
+        /**
+         * Execute a write operation.
          *
          * @param cursor containing items to add/remove
-         * @return {@link WriteResult} indicating success/failure and the number of {@code cursor}
-         *          items that were inserted/updated/deleted in the picker db
+         * @return number of {@code cursor} items that were inserted/updated/deleted in the db
          * @throws {@link IllegalStateException} if no DB transaction is active
          */
         public int execute(@Nullable Cursor cursor) {
@@ -315,21 +317,17 @@ public class PickerDbFacade {
             return mIsLocal;
         }
 
-        String albumId() {
-            return mAlbumId;
-        }
-
         int updateMedia(SQLiteQueryBuilder qb, ContentValues values,
                 String[] selectionArgs) {
             try {
                 if (qb.update(mDatabase, values, /* selection */ null, selectionArgs) > 0) {
                     return SUCCESS;
                 } else {
-                    Log.d(TAG, "Failed to update picker db media. ContentValues: " + values);
+                    Log.v(TAG, "Failed to update picker db media. ContentValues: " + values);
                     return FAIL;
                 }
             } catch (SQLiteConstraintException e) {
-                Log.d(TAG, "Failed to update picker db media. ContentValues: " + values, e);
+                Log.v(TAG, "Failed to update picker db media. ContentValues: " + values, e);
                 return RETRY;
             }
         }
@@ -348,6 +346,41 @@ public class PickerDbFacade {
         }
     }
 
+    /**
+     * Represents an atomic media update operation to the picker database.
+     *
+     * <p>This class is not thread-safe and is meant to be used within a single thread only.
+     */
+    public static final class UpdateMediaOperation extends DbWriteOperation {
+
+        private UpdateMediaOperation(SQLiteDatabase database, boolean isLocal) {
+            super(database, isLocal);
+        }
+
+        /**
+         * Execute a media update operation.
+         *
+         * @param id id of the media to be updated
+         * @param contentValues key-value pairs indicating fields to be updated for the media
+         * @return boolean indicating success/failure of the update
+         * @throws {@link IllegalStateException} if no DB transaction is active
+         */
+        public boolean execute(String id, ContentValues contentValues) {
+            final SQLiteDatabase database = getDatabase();
+            if (!database.inTransaction()) {
+                throw new IllegalStateException("No ongoing DB transaction.");
+            }
+
+            final SQLiteQueryBuilder qb = isLocal() ? QB_MATCH_LOCAL_ONLY : QB_MATCH_CLOUD;
+            return qb.update(database, contentValues, /* selection */ null, new String[] {id}) > 0;
+        }
+
+        @Override
+        int executeInternal(@Nullable Cursor cursor) {
+            throw new UnsupportedOperationException("Cursor updates are not supported.");
+        }
+    }
+
     private static final class AddMediaOperation extends DbWriteOperation {
 
         private AddMediaOperation(SQLiteDatabase database, boolean isLocal) {
@@ -394,11 +427,11 @@ public class PickerDbFacade {
                 if (QB_MATCH_ALL.insert(getDatabase(), values) > 0) {
                     return SUCCESS;
                 } else {
-                    Log.d(TAG, "Failed to insert picker db media. ContentValues: " + values);
+                    Log.v(TAG, "Failed to insert picker db media. ContentValues: " + values);
                     return FAIL;
                 }
             } catch (SQLiteConstraintException e) {
-                Log.d(TAG, "Failed to insert picker db media. ContentValues: " + values, e);
+                Log.v(TAG, "Failed to insert picker db media. ContentValues: " + values, e);
                 return RETRY;
             }
         }
@@ -408,7 +441,7 @@ public class PickerDbFacade {
             int res = insertMedia(values);
             if (res == RETRY) {
                 // Attempt equivalent of CONFLICT_REPLACE resolution
-                Log.d(TAG, "Retrying failed insert as update. ContentValues: " + values);
+                Log.v(TAG, "Retrying failed insert as update. ContentValues: " + values);
                 res = updateMedia(qb, values, selectionArgs);
             }
 
@@ -906,7 +939,7 @@ public class PickerDbFacade {
     private static ContentValues cursorToContentValue(Cursor cursor, boolean isLocal,
             String albumId) {
         final ContentValues values = new ContentValues();
-        if(TextUtils.isEmpty(albumId)) {
+        if (TextUtils.isEmpty(albumId)) {
             values.put(KEY_IS_VISIBLE, 1);
         }
         else {
@@ -955,7 +988,7 @@ public class PickerDbFacade {
                     values.put(KEY_DURATION_MS, cursor.getLong(index));
                     break;
                 case CloudMediaProviderContract.MediaColumns.IS_FAVORITE:
-                    if(TextUtils.isEmpty(albumId)) {
+                    if (TextUtils.isEmpty(albumId)) {
                         values.put(KEY_IS_FAVORITE, cursor.getInt(index));
                     }
                     break;
@@ -1104,25 +1137,35 @@ public class PickerDbFacade {
         return qb;
     }
 
-    private static final class ResetAlbumOperation extends DbWriteOperation {
-        /**
-         * Resets the given cloud or local album_media identified by {@code isLocal} and
-         * {@code albumId}. If {@code albumId} is null, resets all the respective cloud or
-         * local albums.
-         */
+    private abstract static class AlbumWriteOperation extends DbWriteOperation {
+
+        private final String mAlbumId;
+
+        private AlbumWriteOperation(SQLiteDatabase database, boolean isLocal, String albumId) {
+            super(database, isLocal);
+            mAlbumId = albumId;
+        }
+
+        String getAlbumId() {
+            return mAlbumId;
+        }
+    }
+
+    private static final class ResetAlbumOperation extends AlbumWriteOperation {
+
         private ResetAlbumOperation(SQLiteDatabase database, boolean isLocal, String albumId) {
             super(database, isLocal, albumId);
         }
 
         @Override
         int executeInternal(@Nullable Cursor unused) {
-            final String albumId = albumId();
+            final String albumId = getAlbumId();
             final boolean isLocal = isLocal();
 
             final SQLiteQueryBuilder qb = createAlbumMediaQueryBuilder(isLocal);
 
             String[] selectionArgs = null;
-            if(!TextUtils.isEmpty(albumId)) {
+            if (!TextUtils.isEmpty(albumId)) {
                 qb.appendWhereStandalone(WHERE_ALBUM_ID);
                 selectionArgs = new String[]{albumId};
             }
@@ -1132,10 +1175,12 @@ public class PickerDbFacade {
         }
     }
 
-    private static final class AddAlbumMediaOperation extends DbWriteOperation {
+    private static final class AddAlbumMediaOperation extends AlbumWriteOperation {
+
         private AddAlbumMediaOperation(SQLiteDatabase database, boolean isLocal, String albumId) {
             super(database, isLocal, albumId);
-            if(TextUtils.isEmpty(albumId)) {
+
+            if (TextUtils.isEmpty(albumId)) {
                 throw new IllegalArgumentException("Missing albumId.");
             }
         }
@@ -1143,7 +1188,7 @@ public class PickerDbFacade {
         @Override
         int executeInternal(@Nullable Cursor cursor) {
             final boolean isLocal = isLocal();
-            final String albumId = albumId();
+            final String albumId = getAlbumId();
             final SQLiteQueryBuilder qb = createAlbumMediaQueryBuilder(isLocal);
             int counter = 0;
 
@@ -1153,10 +1198,10 @@ public class PickerDbFacade {
                     if (qb.insert(getDatabase(), values) > 0) {
                         counter++;
                     } else {
-                        Log.d(TAG, "Failed to insert album_media. ContentValues: " + values);
+                        Log.v(TAG, "Failed to insert album_media. ContentValues: " + values);
                     }
                 } catch (SQLiteConstraintException e) {
-                    Log.d(TAG, "Failed to insert album_media. ContentValues: " + values, e);
+                    Log.v(TAG, "Failed to insert album_media. ContentValues: " + values, e);
                 }
             }
 
diff --git a/tests/src/com/android/providers/media/IdleServiceTest.java b/tests/src/com/android/providers/media/IdleServiceTest.java
index 39f7e6e5d..3d81b255b 100644
--- a/tests/src/com/android/providers/media/IdleServiceTest.java
+++ b/tests/src/com/android/providers/media/IdleServiceTest.java
@@ -249,9 +249,8 @@ public class IdleServiceTest {
                 assertThat(cr.getCount()).isEqualTo(1);
                 assertThat(cr.moveToFirst()).isNotNull();
                 assertThat(cr.getInt(0)).isEqualTo(_SPECIAL_FORMAT_NONE);
-                // Make sure updating special format column updates GENERATION_MODIFIED;
-                // This is essential for picker db to know which rows were modified.
-                assertThat(cr.getInt(1)).isGreaterThan(initialGenerationModified);
+                // Make sure that updating special format column doesn't update GENERATION_MODIFIED
+                assertThat(cr.getInt(1)).isEqualTo(initialGenerationModified);
             }
         } finally {
             file.delete();
diff --git a/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java b/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java
index 5e13bec9c..0305bbb6c 100644
--- a/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java
+++ b/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java
@@ -23,6 +23,7 @@ import static com.google.common.truth.Truth.assertThat;
 
 import static org.junit.Assert.assertThrows;
 
+import android.content.ContentValues;
 import android.content.Context;
 import android.database.Cursor;
 import android.database.MatrixCursor;
@@ -1014,6 +1015,68 @@ public class PickerDbFacadeTest {
         }
     }
 
+    @Test
+    public void testUpdateMediaSuccess() throws Exception {
+        Cursor localCursor = getMediaCursor(LOCAL_ID, DATE_TAKEN_MS, GENERATION_MODIFIED,
+                /* mediaStoreUri */ null, SIZE_BYTES, VIDEO_MIME_TYPE,
+                STANDARD_MIME_TYPE_EXTENSION, /* isFavorite */ true);
+        try (PickerDbFacade.DbWriteOperation operation =
+                     mFacade.beginAddMediaOperation(LOCAL_PROVIDER)) {
+            operation.execute(localCursor);
+            operation.setSuccess();
+        }
+
+        try (PickerDbFacade.UpdateMediaOperation operation =
+                     mFacade.beginUpdateMediaOperation(LOCAL_PROVIDER)) {
+            ContentValues values = new ContentValues();
+            values.put(PickerDbFacade.KEY_STANDARD_MIME_TYPE_EXTENSION,
+                    MediaColumns.STANDARD_MIME_TYPE_EXTENSION_ANIMATED_WEBP);
+            assertThat(operation.execute(LOCAL_ID, values)).isTrue();
+            operation.setSuccess();
+        }
+
+        try (Cursor cursor = queryMediaAll()) {
+            assertThat(cursor.getCount()).isEqualTo(1);
+
+            // Assert that STANDARD_MIME_TYPE_EXTENSION has been updated
+            cursor.moveToFirst();
+            assertThat(cursor.getInt(cursor.getColumnIndex(
+                    MediaColumns.STANDARD_MIME_TYPE_EXTENSION)))
+                    .isEqualTo(MediaColumns.STANDARD_MIME_TYPE_EXTENSION_ANIMATED_WEBP);
+        }
+    }
+
+    @Test
+    public void testUpdateMediaFailure() throws Exception {
+        Cursor localCursor = getMediaCursor(LOCAL_ID, DATE_TAKEN_MS, GENERATION_MODIFIED,
+                /* mediaStoreUri */ null, SIZE_BYTES, VIDEO_MIME_TYPE,
+                STANDARD_MIME_TYPE_EXTENSION, /* isFavorite */ true);
+        try (PickerDbFacade.DbWriteOperation operation =
+                     mFacade.beginAddMediaOperation(LOCAL_PROVIDER)) {
+            operation.execute(localCursor);
+            operation.setSuccess();
+        }
+
+        try (PickerDbFacade.UpdateMediaOperation operation =
+                     mFacade.beginUpdateMediaOperation(LOCAL_PROVIDER)) {
+            ContentValues values = new ContentValues();
+            values.put(PickerDbFacade.KEY_STANDARD_MIME_TYPE_EXTENSION,
+                    MediaColumns.STANDARD_MIME_TYPE_EXTENSION_ANIMATED_WEBP);
+            assertThat(operation.execute(CLOUD_ID, values)).isFalse();
+            operation.setSuccess();
+        }
+
+        try (Cursor cursor = queryMediaAll()) {
+            assertThat(cursor.getCount()).isEqualTo(1);
+
+            // Assert that STANDARD_MIME_TYPE_EXTENSION is same as before
+            cursor.moveToFirst();
+            assertThat(cursor.getInt(cursor.getColumnIndex(
+                    MediaColumns.STANDARD_MIME_TYPE_EXTENSION)))
+                    .isEqualTo(STANDARD_MIME_TYPE_EXTENSION);
+        }
+    }
+
     private Cursor queryMediaAll() {
         return mFacade.queryMediaForUi(
                 new PickerDbFacade.QueryFilterBuilder(1000).build());