From ab5dae64b9caf51752a095d2f8fbdd6f0e618a59 Mon Sep 17 00:00:00 2001 From: olly Date: Wed, 6 Mar 2019 14:51:03 +0000 Subject: [PATCH] Make sure we handle SQLiteException and other IO errors properly SQLiteException is a runtime exception, which makes it easy to forget to handle it. This change converts SQLiteExceptions into a checked exception, which is then handled appropriately. PiperOrigin-RevId: 237038793 --- .../exoplayer2/demo/DownloadTracker.java | 39 ++-- .../database/DatabaseIOException.java | 31 +++ .../exoplayer2/database/VersionTable.java | 79 +++++--- .../offline/DefaultDownloadIndex.java | 132 +++++++----- .../exoplayer2/offline/DownloadIndex.java | 20 +- .../exoplayer2/offline/DownloadIndexUtil.java | 3 +- .../exoplayer2/offline/DownloadManager.java | 1 + .../cache/CacheFileMetadataIndex.java | 111 ++++++---- .../upstream/cache/CachedContentIndex.java | 189 ++++++++++-------- .../upstream/cache/SimpleCache.java | 107 +++++++--- .../exoplayer2/database/VersionTableTest.java | 8 +- .../offline/DefaultDownloadIndexTest.java | 29 ++- .../offline/DownloadIndexUtilTest.java | 10 +- .../cache/CachedContentIndexTest.java | 5 +- .../upstream/cache/SimpleCacheTest.java | 9 +- 15 files changed, 485 insertions(+), 288 deletions(-) create mode 100644 library/core/src/main/java/com/google/android/exoplayer2/database/DatabaseIOException.java diff --git a/demos/main/src/main/java/com/google/android/exoplayer2/demo/DownloadTracker.java b/demos/main/src/main/java/com/google/android/exoplayer2/demo/DownloadTracker.java index 9afcc20d26..ed12353687 100644 --- a/demos/main/src/main/java/com/google/android/exoplayer2/demo/DownloadTracker.java +++ b/demos/main/src/main/java/com/google/android/exoplayer2/demo/DownloadTracker.java @@ -29,6 +29,7 @@ import com.google.android.exoplayer2.offline.ActionFile; import com.google.android.exoplayer2.offline.DefaultDownloadIndex; import com.google.android.exoplayer2.offline.DownloadAction; import com.google.android.exoplayer2.offline.DownloadHelper; +import com.google.android.exoplayer2.offline.DownloadIndex; import com.google.android.exoplayer2.offline.DownloadManager; import com.google.android.exoplayer2.offline.DownloadService; import com.google.android.exoplayer2.offline.DownloadState; @@ -71,8 +72,8 @@ public class DownloadTracker implements DownloadManager.Listener { private final DataSource.Factory dataSourceFactory; private final CopyOnWriteArraySet listeners; private final HashMap trackedDownloadStates; - private final DefaultDownloadIndex downloadIndex; - private final Handler actionFileIOHandler; + private final DownloadIndex downloadIndex; + private final Handler indexHandler; @Nullable private StartDownloadDialogHelper startDownloadDialogHelper; @@ -83,9 +84,9 @@ public class DownloadTracker implements DownloadManager.Listener { this.downloadIndex = downloadIndex; listeners = new CopyOnWriteArraySet<>(); trackedDownloadStates = new HashMap<>(); - HandlerThread actionFileWriteThread = new HandlerThread("DownloadTracker"); - actionFileWriteThread.start(); - actionFileIOHandler = new Handler(actionFileWriteThread.getLooper()); + HandlerThread indexThread = new HandlerThread("DownloadTracker"); + indexThread.start(); + indexHandler = new Handler(indexThread.getLooper()); loadTrackedActions(); } @@ -163,24 +164,32 @@ public class DownloadTracker implements DownloadManager.Listener { // Internal methods private void loadTrackedActions() { - DownloadStateCursor downloadStates = downloadIndex.getDownloadStates(); - while (downloadStates.moveToNext()) { - DownloadState downloadState = downloadStates.getDownloadState(); - trackedDownloadStates.put(downloadState.uri, downloadState); + try { + DownloadStateCursor downloadStates = downloadIndex.getDownloadStates(); + while (downloadStates.moveToNext()) { + DownloadState downloadState = downloadStates.getDownloadState(); + trackedDownloadStates.put(downloadState.uri, downloadState); + } + downloadStates.close(); + } catch (IOException e) { + Log.w(TAG, "Failed to query download states", e); } - downloadStates.close(); } private void handleTrackedDownloadStateChanged(DownloadState downloadState) { for (Listener listener : listeners) { listener.onDownloadsChanged(); } - actionFileIOHandler.post( + indexHandler.post( () -> { - if (downloadState.state == DownloadState.STATE_REMOVED) { - downloadIndex.removeDownloadState(downloadState.id); - } else { - downloadIndex.putDownloadState(downloadState); + try { + if (downloadState.state == DownloadState.STATE_REMOVED) { + downloadIndex.removeDownloadState(downloadState.id); + } else { + downloadIndex.putDownloadState(downloadState); + } + } catch (IOException e) { + // TODO: This whole method is going away in cr/232854678. } }); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/database/DatabaseIOException.java b/library/core/src/main/java/com/google/android/exoplayer2/database/DatabaseIOException.java new file mode 100644 index 0000000000..b0fd1cf2f0 --- /dev/null +++ b/library/core/src/main/java/com/google/android/exoplayer2/database/DatabaseIOException.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.android.exoplayer2.database; + +import android.database.SQLException; +import java.io.IOException; + +/** An {@link IOException} whose cause is an {@link SQLException}. */ +public final class DatabaseIOException extends IOException { + + public DatabaseIOException(SQLException cause) { + super(cause); + } + + public DatabaseIOException(SQLException cause, String message) { + super(message, cause); + } +} diff --git a/library/core/src/main/java/com/google/android/exoplayer2/database/VersionTable.java b/library/core/src/main/java/com/google/android/exoplayer2/database/VersionTable.java index e0a4210136..be367d2f22 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/database/VersionTable.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/database/VersionTable.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.database; import android.content.ContentValues; import android.database.Cursor; import android.database.DatabaseUtils; +import android.database.SQLException; import android.database.sqlite.SQLiteDatabase; import androidx.annotation.IntDef; import androidx.annotation.VisibleForTesting; @@ -78,15 +79,21 @@ public final class VersionTable { * @param feature The feature. * @param instanceUid The unique identifier of the instance of the feature. * @param version The version. + * @throws DatabaseIOException If an error occurs executing the SQL. */ public static void setVersion( - SQLiteDatabase writableDatabase, @Feature int feature, String instanceUid, int version) { - writableDatabase.execSQL(SQL_CREATE_TABLE_IF_NOT_EXISTS); - ContentValues values = new ContentValues(); - values.put(COLUMN_FEATURE, feature); - values.put(COLUMN_INSTANCE_UID, instanceUid); - values.put(COLUMN_VERSION, version); - writableDatabase.replace(TABLE_NAME, /* nullColumnHack= */ null, values); + SQLiteDatabase writableDatabase, @Feature int feature, String instanceUid, int version) + throws DatabaseIOException { + try { + writableDatabase.execSQL(SQL_CREATE_TABLE_IF_NOT_EXISTS); + ContentValues values = new ContentValues(); + values.put(COLUMN_FEATURE, feature); + values.put(COLUMN_INSTANCE_UID, instanceUid); + values.put(COLUMN_VERSION, version); + writableDatabase.replaceOrThrow(TABLE_NAME, /* nullColumnHack= */ null, values); + } catch (SQLException e) { + throw new DatabaseIOException(e); + } } /** @@ -95,16 +102,22 @@ public final class VersionTable { * @param writableDatabase The database to update. * @param feature The feature. * @param instanceUid The unique identifier of the instance of the feature. + * @throws DatabaseIOException If an error occurs executing the SQL. */ public static void removeVersion( - SQLiteDatabase writableDatabase, @Feature int feature, String instanceUid) { - if (!tableExists(writableDatabase, TABLE_NAME)) { - return; + SQLiteDatabase writableDatabase, @Feature int feature, String instanceUid) + throws DatabaseIOException { + try { + if (!tableExists(writableDatabase, TABLE_NAME)) { + return; + } + writableDatabase.delete( + TABLE_NAME, + WHERE_FEATURE_AND_INSTANCE_UID_EQUALS, + featureAndInstanceUidArguments(feature, instanceUid)); + } catch (SQLException e) { + throw new DatabaseIOException(e); } - writableDatabase.delete( - TABLE_NAME, - WHERE_FEATURE_AND_INSTANCE_UID_EQUALS, - featureAndInstanceUidArguments(feature, instanceUid)); } /** @@ -115,25 +128,31 @@ public final class VersionTable { * @param feature The feature. * @param instanceUid The unique identifier of the instance of the feature. * @return The version, or {@link #VERSION_UNSET} if no version is set. + * @throws DatabaseIOException If an error occurs executing the SQL. */ - public static int getVersion(SQLiteDatabase database, @Feature int feature, String instanceUid) { - if (!tableExists(database, TABLE_NAME)) { - return VERSION_UNSET; - } - try (Cursor cursor = - database.query( - TABLE_NAME, - new String[] {COLUMN_VERSION}, - WHERE_FEATURE_AND_INSTANCE_UID_EQUALS, - featureAndInstanceUidArguments(feature, instanceUid), - /* groupBy= */ null, - /* having= */ null, - /* orderBy= */ null)) { - if (cursor.getCount() == 0) { + public static int getVersion(SQLiteDatabase database, @Feature int feature, String instanceUid) + throws DatabaseIOException { + try { + if (!tableExists(database, TABLE_NAME)) { return VERSION_UNSET; } - cursor.moveToNext(); - return cursor.getInt(/* COLUMN_VERSION index */ 0); + try (Cursor cursor = + database.query( + TABLE_NAME, + new String[] {COLUMN_VERSION}, + WHERE_FEATURE_AND_INSTANCE_UID_EQUALS, + featureAndInstanceUidArguments(feature, instanceUid), + /* groupBy= */ null, + /* having= */ null, + /* orderBy= */ null)) { + if (cursor.getCount() == 0) { + return VERSION_UNSET; + } + cursor.moveToNext(); + return cursor.getInt(/* COLUMN_VERSION index */ 0); + } + } catch (SQLException e) { + throw new DatabaseIOException(e); } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java index f224258495..402003ea7f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java @@ -17,10 +17,13 @@ package com.google.android.exoplayer2.offline; import android.content.ContentValues; import android.database.Cursor; +import android.database.SQLException; import android.database.sqlite.SQLiteDatabase; +import android.database.sqlite.SQLiteException; import android.net.Uri; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import com.google.android.exoplayer2.database.DatabaseIOException; import com.google.android.exoplayer2.database.DatabaseProvider; import com.google.android.exoplayer2.database.VersionTable; import com.google.android.exoplayer2.util.Assertions; @@ -152,7 +155,7 @@ public final class DefaultDownloadIndex implements DownloadIndex { @Override @Nullable - public DownloadState getDownloadState(String id) { + public DownloadState getDownloadState(String id) throws DatabaseIOException { ensureInitialized(); try (Cursor cursor = getCursor(WHERE_ID_EQUALS, new String[] {id})) { if (cursor.getCount() == 0) { @@ -162,83 +165,102 @@ public final class DefaultDownloadIndex implements DownloadIndex { DownloadState downloadState = getDownloadStateForCurrentRow(cursor); Assertions.checkState(id.equals(downloadState.id)); return downloadState; + } catch (SQLiteException e) { + throw new DatabaseIOException(e); } } @Override - public DownloadStateCursor getDownloadStates(@DownloadState.State int... states) { + public DownloadStateCursor getDownloadStates(@DownloadState.State int... states) + throws DatabaseIOException { ensureInitialized(); - String selection = null; - if (states.length > 0) { - StringBuilder selectionBuilder = new StringBuilder(); - selectionBuilder.append(COLUMN_STATE).append(" IN ("); - for (int i = 0; i < states.length; i++) { - if (i > 0) { - selectionBuilder.append(','); + try { + String selection = null; + if (states.length > 0) { + StringBuilder selectionBuilder = new StringBuilder(); + selectionBuilder.append(COLUMN_STATE).append(" IN ("); + for (int i = 0; i < states.length; i++) { + if (i > 0) { + selectionBuilder.append(','); + } + selectionBuilder.append(states[i]); } - selectionBuilder.append(states[i]); + selectionBuilder.append(')'); + selection = selectionBuilder.toString(); } - selectionBuilder.append(')'); - selection = selectionBuilder.toString(); + Cursor cursor = getCursor(selection, /* selectionArgs= */ null); + return new DownloadStateCursorImpl(cursor); + } catch (SQLiteException e) { + throw new DatabaseIOException(e); } - Cursor cursor = getCursor(selection, /* selectionArgs= */ null); - return new DownloadStateCursorImpl(cursor); } @Override - public void putDownloadState(DownloadState downloadState) { + public void putDownloadState(DownloadState downloadState) throws DatabaseIOException { ensureInitialized(); - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - ContentValues values = new ContentValues(); - values.put(COLUMN_ID, downloadState.id); - values.put(COLUMN_TYPE, downloadState.type); - values.put(COLUMN_URI, downloadState.uri.toString()); - values.put(COLUMN_CACHE_KEY, downloadState.cacheKey); - values.put(COLUMN_STATE, downloadState.state); - values.put(COLUMN_DOWNLOAD_PERCENTAGE, downloadState.downloadPercentage); - values.put(COLUMN_DOWNLOADED_BYTES, downloadState.downloadedBytes); - values.put(COLUMN_TOTAL_BYTES, downloadState.totalBytes); - values.put(COLUMN_FAILURE_REASON, downloadState.failureReason); - values.put(COLUMN_STOP_FLAGS, downloadState.stopFlags); - values.put(COLUMN_NOT_MET_REQUIREMENTS, downloadState.notMetRequirements); - values.put(COLUMN_MANUAL_STOP_REASON, downloadState.manualStopReason); - values.put(COLUMN_START_TIME_MS, downloadState.startTimeMs); - values.put(COLUMN_UPDATE_TIME_MS, downloadState.updateTimeMs); - values.put(COLUMN_STREAM_KEYS, encodeStreamKeys(downloadState.streamKeys)); - values.put(COLUMN_CUSTOM_METADATA, downloadState.customMetadata); - writableDatabase.replace(TABLE_NAME, /* nullColumnHack= */ null, values); + try { + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + ContentValues values = new ContentValues(); + values.put(COLUMN_ID, downloadState.id); + values.put(COLUMN_TYPE, downloadState.type); + values.put(COLUMN_URI, downloadState.uri.toString()); + values.put(COLUMN_CACHE_KEY, downloadState.cacheKey); + values.put(COLUMN_STATE, downloadState.state); + values.put(COLUMN_DOWNLOAD_PERCENTAGE, downloadState.downloadPercentage); + values.put(COLUMN_DOWNLOADED_BYTES, downloadState.downloadedBytes); + values.put(COLUMN_TOTAL_BYTES, downloadState.totalBytes); + values.put(COLUMN_FAILURE_REASON, downloadState.failureReason); + values.put(COLUMN_STOP_FLAGS, downloadState.stopFlags); + values.put(COLUMN_NOT_MET_REQUIREMENTS, downloadState.notMetRequirements); + values.put(COLUMN_MANUAL_STOP_REASON, downloadState.manualStopReason); + values.put(COLUMN_START_TIME_MS, downloadState.startTimeMs); + values.put(COLUMN_UPDATE_TIME_MS, downloadState.updateTimeMs); + values.put(COLUMN_STREAM_KEYS, encodeStreamKeys(downloadState.streamKeys)); + values.put(COLUMN_CUSTOM_METADATA, downloadState.customMetadata); + writableDatabase.replaceOrThrow(TABLE_NAME, /* nullColumnHack= */ null, values); + } catch (SQLiteException e) { + throw new DatabaseIOException(e); + } } @Override - public void removeDownloadState(String id) { + public void removeDownloadState(String id) throws DatabaseIOException { ensureInitialized(); - databaseProvider.getWritableDatabase().delete(TABLE_NAME, WHERE_ID_EQUALS, new String[] {id}); + try { + databaseProvider.getWritableDatabase().delete(TABLE_NAME, WHERE_ID_EQUALS, new String[] {id}); + } catch (SQLiteException e) { + throw new DatabaseIOException(e); + } } - private void ensureInitialized() { + private void ensureInitialized() throws DatabaseIOException { if (initialized) { return; } - SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase(); - int version = - VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE, INSTANCE_UID); - if (version == VersionTable.VERSION_UNSET || version > TABLE_VERSION) { - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.beginTransaction(); - try { - VersionTable.setVersion( - writableDatabase, VersionTable.FEATURE_OFFLINE, INSTANCE_UID, TABLE_VERSION); - writableDatabase.execSQL(SQL_DROP_TABLE_IF_EXISTS); - writableDatabase.execSQL(SQL_CREATE_TABLE); - writableDatabase.setTransactionSuccessful(); - } finally { - writableDatabase.endTransaction(); + try { + SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase(); + int version = + VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE, INSTANCE_UID); + if (version == VersionTable.VERSION_UNSET || version > TABLE_VERSION) { + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.beginTransaction(); + try { + VersionTable.setVersion( + writableDatabase, VersionTable.FEATURE_OFFLINE, INSTANCE_UID, TABLE_VERSION); + writableDatabase.execSQL(SQL_DROP_TABLE_IF_EXISTS); + writableDatabase.execSQL(SQL_CREATE_TABLE); + writableDatabase.setTransactionSuccessful(); + } finally { + writableDatabase.endTransaction(); + } + } else if (version < TABLE_VERSION) { + // There is no previous version currently. + throw new IllegalStateException(); } - } else if (version < TABLE_VERSION) { - // There is no previous version currently. - throw new IllegalStateException(); + initialized = true; + } catch (SQLException e) { + throw new DatabaseIOException(e); } - initialized = true; } private Cursor getCursor(@Nullable String selection, @Nullable String[] selectionArgs) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndex.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndex.java index 77840cc65b..8d997e07ff 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndex.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndex.java @@ -16,9 +16,10 @@ package com.google.android.exoplayer2.offline; import androidx.annotation.Nullable; +import java.io.IOException; /** Persists {@link DownloadState}s. */ -interface DownloadIndex { +public interface DownloadIndex { /** * Returns the {@link DownloadState} with the given {@code id}, or null. @@ -26,25 +27,32 @@ interface DownloadIndex { * @param id ID of a {@link DownloadState}. * @return The {@link DownloadState} with the given {@code id}, or null if a download state with * this id doesn't exist. + * @throws IOException If an error occurs reading the state. */ @Nullable - DownloadState getDownloadState(String id); + DownloadState getDownloadState(String id) throws IOException; /** * Returns a {@link DownloadStateCursor} to {@link DownloadState}s with the given {@code states}. * * @param states Returns only the {@link DownloadState}s with this states. If empty, returns all. * @return A cursor to {@link DownloadState}s with the given {@code states}. + * @throws IOException If an error occurs reading the state. */ - DownloadStateCursor getDownloadStates(@DownloadState.State int... states); + DownloadStateCursor getDownloadStates(@DownloadState.State int... states) throws IOException; /** * Adds or replaces a {@link DownloadState}. * * @param downloadState The {@link DownloadState} to be added. + * @throws IOException If an error occurs setting the state. */ - void putDownloadState(DownloadState downloadState); + void putDownloadState(DownloadState downloadState) throws IOException; - /** Removes the {@link DownloadState} with the given {@code id}. */ - void removeDownloadState(String id); + /** + * Removes the {@link DownloadState} with the given {@code id}. + * + * @throws IOException If an error occurs removing the state. + */ + void removeDownloadState(String id) throws IOException; } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java index c93fa2135b..ef1418b24f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadIndexUtil.java @@ -69,9 +69,10 @@ public final class DownloadIndexUtil { * @param downloadIndex The action is converted to {@link DownloadState} and stored in this index. * @param id A nullable custom download id which overwrites {@link DownloadAction#id}. * @param action The action to be stored in {@link DownloadIndex}. + * @throws IOException If an error occurs storing the state in the {@link DownloadIndex}. */ public static void addAction( - DownloadIndex downloadIndex, @Nullable String id, DownloadAction action) { + DownloadIndex downloadIndex, @Nullable String id, DownloadAction action) throws IOException { DownloadState downloadState = downloadIndex.getDownloadState(id != null ? id : action.id); if (downloadState != null) { downloadState = downloadState.mergeAction(action); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java index f007f8c9ac..3415db8ead 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloadManager.java @@ -810,6 +810,7 @@ public final class DownloadManager { setState(STATE_REMOVED); } else { // STATE_DOWNLOADING if (error != null) { + Log.e(TAG, "Download failed: " + downloadState.id, error); failureReason = FAILURE_REASON_UNKNOWN; setState(STATE_FAILED); } else { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheFileMetadataIndex.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheFileMetadataIndex.java index a370311939..2b5f116d1f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheFileMetadataIndex.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheFileMetadataIndex.java @@ -17,7 +17,9 @@ package com.google.android.exoplayer2.upstream.cache; import android.content.ContentValues; import android.database.Cursor; +import android.database.SQLException; import android.database.sqlite.SQLiteDatabase; +import com.google.android.exoplayer2.database.DatabaseIOException; import com.google.android.exoplayer2.database.DatabaseProvider; import com.google.android.exoplayer2.database.VersionTable; import com.google.android.exoplayer2.util.Assertions; @@ -63,36 +65,48 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; this.databaseProvider = databaseProvider; } - /** Initializes the index for the given cache UID. */ - public void initialize(long uid) { - String hexUid = Long.toHexString(uid); - tableName = TABLE_PREFIX + hexUid; - SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase(); - int version = - VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_CACHE_FILE_METADATA, hexUid); - if (version == VersionTable.VERSION_UNSET || version > TABLE_VERSION) { - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.beginTransaction(); - try { - VersionTable.setVersion( - writableDatabase, VersionTable.FEATURE_CACHE_FILE_METADATA, hexUid, TABLE_VERSION); - writableDatabase.execSQL("DROP TABLE IF EXISTS " + tableName); - writableDatabase.execSQL("CREATE TABLE " + tableName + " " + TABLE_SCHEMA); - writableDatabase.setTransactionSuccessful(); - } finally { - writableDatabase.endTransaction(); + /** + * Initializes the index for the given cache UID. + * + * @throws DatabaseIOException If an error occurs initializing the index. + */ + public void initialize(long uid) throws DatabaseIOException { + try { + String hexUid = Long.toHexString(uid); + tableName = TABLE_PREFIX + hexUid; + SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase(); + int version = + VersionTable.getVersion( + readableDatabase, VersionTable.FEATURE_CACHE_FILE_METADATA, hexUid); + if (version == VersionTable.VERSION_UNSET || version > TABLE_VERSION) { + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.beginTransaction(); + try { + VersionTable.setVersion( + writableDatabase, VersionTable.FEATURE_CACHE_FILE_METADATA, hexUid, TABLE_VERSION); + writableDatabase.execSQL("DROP TABLE IF EXISTS " + tableName); + writableDatabase.execSQL("CREATE TABLE " + tableName + " " + TABLE_SCHEMA); + writableDatabase.setTransactionSuccessful(); + } finally { + writableDatabase.endTransaction(); + } + } else if (version < TABLE_VERSION) { + // There is no previous version currently. + throw new IllegalStateException(); } - } else if (version < TABLE_VERSION) { - // There is no previous version currently. - throw new IllegalStateException(); + } catch (SQLException e) { + throw new DatabaseIOException(e); } } /** * Returns all file metadata keyed by file name. The returned map is mutable and may be modified * by the caller. + * + * @return The file metadata keyed by file name. + * @throws DatabaseIOException If an error occurs loading the metadata. */ - public Map getAll() { + public Map getAll() throws DatabaseIOException { try (Cursor cursor = getCursor()) { Map fileMetadata = new HashMap<>(cursor.getCount()); while (cursor.moveToNext()) { @@ -102,6 +116,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; fileMetadata.put(name, new CacheFileMetadata(length, lastAccessTimestamp)); } return fileMetadata; + } catch (SQLException e) { + throw new DatabaseIOException(e); } } @@ -111,44 +127,59 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; * @param name The name of the file. * @param length The file length. * @param lastAccessTimestamp The file last access timestamp. + * @throws DatabaseIOException If an error occurs setting the metadata. */ - public void set(String name, long length, long lastAccessTimestamp) { + public void set(String name, long length, long lastAccessTimestamp) throws DatabaseIOException { Assertions.checkNotNull(tableName); - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - ContentValues values = new ContentValues(); - values.put(COLUMN_NAME, name); - values.put(COLUMN_LENGTH, length); - values.put(COLUMN_LAST_ACCESS_TIMESTAMP, lastAccessTimestamp); - writableDatabase.replace(tableName, /* nullColumnHack= */ null, values); + try { + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + ContentValues values = new ContentValues(); + values.put(COLUMN_NAME, name); + values.put(COLUMN_LENGTH, length); + values.put(COLUMN_LAST_ACCESS_TIMESTAMP, lastAccessTimestamp); + writableDatabase.replaceOrThrow(tableName, /* nullColumnHack= */ null, values); + } catch (SQLException e) { + throw new DatabaseIOException(e); + } } /** * Removes metadata. * * @param name The name of the file whose metadata is to be removed. + * @throws DatabaseIOException If an error occurs removing the metadata. */ - public void remove(String name) { + public void remove(String name) throws DatabaseIOException { Assertions.checkNotNull(tableName); - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.delete(tableName, WHERE_NAME_EQUALS, new String[] {name}); + try { + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.delete(tableName, WHERE_NAME_EQUALS, new String[] {name}); + } catch (SQLException e) { + throw new DatabaseIOException(e); + } } /** * Removes metadata. * * @param names The names of the files whose metadata is to be removed. + * @throws DatabaseIOException If an error occurs removing the metadata. */ - public void removeAll(Set names) { + public void removeAll(Set names) throws DatabaseIOException { Assertions.checkNotNull(tableName); - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.beginTransaction(); try { - for (String name : names) { - writableDatabase.delete(tableName, WHERE_NAME_EQUALS, new String[] {name}); + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.beginTransaction(); + try { + for (String name : names) { + writableDatabase.delete(tableName, WHERE_NAME_EQUALS, new String[] {name}); + } + writableDatabase.setTransactionSuccessful(); + } finally { + writableDatabase.endTransaction(); } - writableDatabase.setTransactionSuccessful(); - } finally { - writableDatabase.endTransaction(); + } catch (SQLException e) { + throw new DatabaseIOException(e); } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndex.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndex.java index f14e7cef5b..c25c88a361 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndex.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndex.java @@ -18,15 +18,16 @@ package com.google.android.exoplayer2.upstream.cache; import android.annotation.SuppressLint; import android.content.ContentValues; import android.database.Cursor; +import android.database.SQLException; import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteException; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import android.util.SparseArray; import android.util.SparseBooleanArray; +import com.google.android.exoplayer2.database.DatabaseIOException; import com.google.android.exoplayer2.database.DatabaseProvider; import com.google.android.exoplayer2.database.VersionTable; -import com.google.android.exoplayer2.upstream.cache.Cache.CacheException; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.AtomicFile; import com.google.android.exoplayer2.util.ReusableBufferedOutputStream; @@ -160,28 +161,23 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } /** - * Loads the index file for the given cache UID. + * Loads the index data for the given cache UID. * * @param uid The UID of the cache whose index is to be loaded. + * @throws IOException If an error occurs initializing the index data. */ - public void initialize(long uid) { + public void initialize(long uid) throws IOException { storage.initialize(uid); if (previousStorage != null) { previousStorage.initialize(uid); } if (!storage.exists() && previousStorage != null && previousStorage.exists()) { // Copy from previous storage into current storage. - loadFrom(previousStorage); - try { - storage.storeFully(keyToContent); - } catch (CacheException e) { - // We failed to copy into current storage, so keep using previous storage. - storage = previousStorage; - previousStorage = null; - } + previousStorage.load(keyToContent, idToKey); + storage.storeFully(keyToContent); } else { // Load from the current storage. - loadFrom(storage); + storage.load(keyToContent, idToKey); } if (previousStorage != null) { previousStorage.delete(); @@ -189,8 +185,12 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } } - /** Stores the index data to index file if there is a change. */ - public void store() throws CacheException { + /** + * Stores the index data to index file if there is a change. + * + * @throws IOException If an error occurs storing the index data. + */ + public void store() throws IOException { storage.storeIncremental(keyToContent); // Make ids that were removed since the index was last stored eligible for re-use. int removedIdCount = removedIds.size(); @@ -286,14 +286,6 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; return cachedContent != null ? cachedContent.getMetadata() : DefaultContentMetadata.EMPTY; } - /** Loads the index from the specified storage. */ - private void loadFrom(Storage storage) { - if (!storage.load(keyToContent, idToKey)) { - keyToContent.clear(); - idToKey.clear(); - } - } - private CachedContent addNew(String key) { int id = getNewId(idToKey); CachedContent cachedContent = new CachedContent(id, key); @@ -341,7 +333,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; * * @param input Input stream to read from. * @return a {@link DefaultContentMetadata} instance. - * @throws IOException If an error occurs during reading from input. + * @throws IOException If an error occurs during reading from the input. */ private static DefaultContentMetadata readContentMetadata(DataInputStream input) throws IOException { @@ -374,7 +366,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; * Serializes itself to a {@link DataOutputStream}. * * @param output Output stream to store the values. - * @throws IOException If an error occurs during writing values to output. + * @throws IOException If an error occurs writing to the output. */ private static void writeContentMetadata(DefaultContentMetadata metadata, DataOutputStream output) throws IOException { @@ -394,30 +386,43 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; /** Initializes the storage for the given cache UID. */ void initialize(long uid); - /** Returns whether the persisted index exists. */ - boolean exists(); + /** + * Returns whether the persisted index exists. + * + * @throws IOException If an error occurs determining whether the persisted index exists. + */ + boolean exists() throws IOException; - /** Deletes the persisted index. */ - void delete(); + /** + * Deletes the persisted index. + * + * @throws IOException If an error occurs deleting the index. + */ + void delete() throws IOException; /** * Loads the persisted index into {@code content} and {@code idToKey}, creating it if it doesn't * already exist. * + *

If the persisted index is in a permanently bad state (i.e. all further attempts to load it + * are also expected to fail) then it will be deleted and the call will return successfully. For + * transient failures, {@link IOException} will be thrown. + * * @param content The key to content map to populate with persisted data. * @param idToKey The id to key map to populate with persisted data. - * @return Whether the load was successful. + * @throws IOException If an error occurs loading the index. */ - boolean load(HashMap content, SparseArray<@NullableType String> idToKey); + void load(HashMap content, SparseArray<@NullableType String> idToKey) + throws IOException; /** * Writes the persisted index, creating it if it doesn't already exist and replacing any * existing content if it does. * * @param content The key to content map to persist. - * @throws CacheException If an error occurs persisting the index. + * @throws IOException If an error occurs persisting the index. */ - void storeFully(HashMap content) throws CacheException; + void storeFully(HashMap content) throws IOException; /** * Ensures incremental changes to the index since the initial {@link #initialize(long)} or last @@ -425,9 +430,9 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; * changes via {@link #onUpdate(CachedContent)} and {@link #onRemove(CachedContent)}. * * @param content The key to content map to persist. - * @throws CacheException If an error occurs persisting the index. + * @throws IOException If an error occurs persisting the index. */ - void storeIncremental(HashMap content) throws CacheException; + void storeIncremental(HashMap content) throws IOException; /** * Called when a {@link CachedContent} is added or updated. @@ -493,24 +498,24 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } @Override - public boolean load( + public void load( HashMap content, SparseArray<@NullableType String> idToKey) { Assertions.checkState(!changed); if (!readFile(content, idToKey)) { + content.clear(); + idToKey.clear(); atomicFile.delete(); - return false; } - return true; } @Override - public void storeFully(HashMap content) throws CacheException { + public void storeFully(HashMap content) throws IOException { writeFile(content); changed = false; } @Override - public void storeIncremental(HashMap content) throws CacheException { + public void storeIncremental(HashMap content) throws IOException { if (!changed) { return; } @@ -529,6 +534,10 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; private boolean readFile( HashMap content, SparseArray<@NullableType String> idToKey) { + if (!atomicFile.exists()) { + return true; + } + DataInputStream input = null; try { InputStream inputStream = new BufferedInputStream(atomicFile.openRead()); @@ -579,7 +588,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; return true; } - private void writeFile(HashMap content) throws CacheException { + private void writeFile(HashMap content) throws IOException { DataOutputStream output = null; try { OutputStream outputStream = atomicFile.startWrite(); @@ -619,8 +628,6 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; // Avoid calling close twice. Duplicate CipherOutputStream.close calls did // not used to be no-ops: https://android-review.googlesource.com/#/c/272799/ output = null; - } catch (IOException e) { - throw new CacheException(e); } finally { Util.closeQuietly(output); } @@ -722,7 +729,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } @Override - public boolean exists() { + public boolean exists() throws DatabaseIOException { return VersionTable.getVersion( databaseProvider.getReadableDatabase(), VersionTable.FEATURE_CACHE_CONTENT_METADATA, @@ -731,22 +738,27 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; } @Override - public void delete() { - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.beginTransaction(); + public void delete() throws DatabaseIOException { try { - VersionTable.removeVersion( - writableDatabase, VersionTable.FEATURE_CACHE_CONTENT_METADATA, hexUid); - dropTable(writableDatabase); - writableDatabase.setTransactionSuccessful(); - } finally { - writableDatabase.endTransaction(); + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.beginTransaction(); + try { + VersionTable.removeVersion( + writableDatabase, VersionTable.FEATURE_CACHE_CONTENT_METADATA, hexUid); + dropTable(writableDatabase); + writableDatabase.setTransactionSuccessful(); + } finally { + writableDatabase.endTransaction(); + } + } catch (SQLException e) { + throw new DatabaseIOException(e); } } @Override - public boolean load( - HashMap content, SparseArray<@NullableType String> idToKey) { + public void load( + HashMap content, SparseArray<@NullableType String> idToKey) + throws IOException { Assertions.checkState(pendingUpdates.size() == 0); try { int version = @@ -783,52 +795,57 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; idToKey.put(cachedContent.id, cachedContent.key); } } - return true; - } catch (IOException | SQLiteException e) { - return false; + } catch (SQLiteException e) { + content.clear(); + idToKey.clear(); + throw new DatabaseIOException(e); } } @Override - public void storeFully(HashMap content) throws CacheException { - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.beginTransaction(); + public void storeFully(HashMap content) throws IOException { try { - initializeTable(writableDatabase); - for (CachedContent cachedContent : content.values()) { - addOrUpdateRow(writableDatabase, cachedContent); + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.beginTransaction(); + try { + initializeTable(writableDatabase); + for (CachedContent cachedContent : content.values()) { + addOrUpdateRow(writableDatabase, cachedContent); + } + writableDatabase.setTransactionSuccessful(); + pendingUpdates.clear(); + } finally { + writableDatabase.endTransaction(); } - writableDatabase.setTransactionSuccessful(); - pendingUpdates.clear(); - } catch (IOException | SQLiteException e) { - throw new CacheException(e); - } finally { - writableDatabase.endTransaction(); + } catch (SQLException e) { + throw new DatabaseIOException(e); } } @Override - public void storeIncremental(HashMap content) throws CacheException { + public void storeIncremental(HashMap content) throws IOException { if (pendingUpdates.size() == 0) { return; } - SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); - writableDatabase.beginTransaction(); try { - for (int i = 0; i < pendingUpdates.size(); i++) { - CachedContent cachedContent = pendingUpdates.valueAt(i); - if (cachedContent == null) { - deleteRow(writableDatabase, pendingUpdates.keyAt(i)); - } else { - addOrUpdateRow(writableDatabase, cachedContent); + SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); + writableDatabase.beginTransaction(); + try { + for (int i = 0; i < pendingUpdates.size(); i++) { + CachedContent cachedContent = pendingUpdates.valueAt(i); + if (cachedContent == null) { + deleteRow(writableDatabase, pendingUpdates.keyAt(i)); + } else { + addOrUpdateRow(writableDatabase, cachedContent); + } } + writableDatabase.setTransactionSuccessful(); + pendingUpdates.clear(); + } finally { + writableDatabase.endTransaction(); } - writableDatabase.setTransactionSuccessful(); - pendingUpdates.clear(); - } catch (IOException | SQLiteException e) { - throw new CacheException(e); - } finally { - writableDatabase.endTransaction(); + } catch (SQLException e) { + throw new DatabaseIOException(e); } } @@ -855,7 +872,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; /* orderBy= */ null); } - private void initializeTable(SQLiteDatabase writableDatabase) { + private void initializeTable(SQLiteDatabase writableDatabase) throws DatabaseIOException { VersionTable.setVersion( writableDatabase, VersionTable.FEATURE_CACHE_CONTENT_METADATA, hexUid, TABLE_VERSION); dropTable(writableDatabase); @@ -880,7 +897,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; values.put(COLUMN_ID, cachedContent.id); values.put(COLUMN_KEY, cachedContent.key); values.put(COLUMN_METADATA, data); - writableDatabase.replace(tableName, /* nullColumnHack= */ null, values); + writableDatabase.replaceOrThrow(tableName, /* nullColumnHack= */ null, values); } } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java index 888f1a1696..a0fde79bee 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/SimpleCache.java @@ -33,6 +33,7 @@ import java.util.NavigableSet; import java.util.Random; import java.util.Set; import java.util.TreeSet; +import org.checkerframework.checker.nullness.qual.MonotonicNonNull; /** * A {@link Cache} implementation that maintains an in-memory representation. Note, only one @@ -65,6 +66,7 @@ public final class SimpleCache implements Cache { private long uid; private long totalSpace; private boolean released; + @MonotonicNonNull private CacheException initializationException; /** * Returns whether {@code cacheFolder} is locked by a {@link SimpleCache} instance. To unlock the @@ -218,6 +220,17 @@ public final class SimpleCache implements Cache { conditionVariable.block(); } + /** + * Checks whether the cache was initialized successfully. + * + * @throws CacheException If an error occurred during initialization. + */ + public synchronized void checkInitialization() throws CacheException { + if (initializationException != null) { + throw initializationException; + } + } + @Override public synchronized void release() { if (released) { @@ -227,7 +240,7 @@ public final class SimpleCache implements Cache { removeStaleSpans(); try { contentIndex.store(); - } catch (CacheException e) { + } catch (IOException e) { Log.e(TAG, "Storing index file failed", e); } finally { unlockFolder(cacheDir); @@ -286,6 +299,9 @@ public final class SimpleCache implements Cache { @Override public synchronized SimpleCacheSpan startReadWrite(String key, long position) throws InterruptedException, CacheException { + Assertions.checkState(!released); + checkInitialization(); + while (true) { SimpleCacheSpan span = startReadWriteNonBlocking(key, position); if (span != null) { @@ -301,9 +317,12 @@ public final class SimpleCache implements Cache { } @Override - public synchronized @Nullable SimpleCacheSpan startReadWriteNonBlocking( - String key, long position) { + @Nullable + public synchronized SimpleCacheSpan startReadWriteNonBlocking(String key, long position) + throws CacheException { Assertions.checkState(!released); + checkInitialization(); + SimpleCacheSpan span = getSpan(key, position); // Read case. @@ -313,7 +332,11 @@ public final class SimpleCache implements Cache { long lastAccessTimestamp = System.currentTimeMillis(); boolean updateFile = false; if (fileIndex != null) { - fileIndex.set(fileName, length, lastAccessTimestamp); + try { + fileIndex.set(fileName, length, lastAccessTimestamp); + } catch (IOException e) { + throw new CacheException(e); + } } else { // Updating the file itself to incorporate the new last access timestamp is much slower than // updating the file index. Hence we only update the file if we don't have a file index. @@ -337,8 +360,10 @@ public final class SimpleCache implements Cache { } @Override - public synchronized File startFile(String key, long position, long length) { + public synchronized File startFile(String key, long position, long length) throws CacheException { Assertions.checkState(!released); + checkInitialization(); + CachedContent cachedContent = contentIndex.get(key); Assertions.checkNotNull(cachedContent); Assertions.checkState(cachedContent.isLocked()); @@ -368,10 +393,9 @@ public final class SimpleCache implements Cache { return; } - SimpleCacheSpan span = SimpleCacheSpan.createCacheEntry(file, length, contentIndex); - Assertions.checkState(span != null); - CachedContent cachedContent = contentIndex.get(span.key); - Assertions.checkNotNull(cachedContent); + SimpleCacheSpan span = + Assertions.checkNotNull(SimpleCacheSpan.createCacheEntry(file, length, contentIndex)); + CachedContent cachedContent = Assertions.checkNotNull(contentIndex.get(span.key)); Assertions.checkState(cachedContent.isLocked()); // Check if the span conflicts with the set content length @@ -381,10 +405,19 @@ public final class SimpleCache implements Cache { } if (fileIndex != null) { - fileIndex.set(file.getName(), span.length, span.lastAccessTimestamp); + String fileName = file.getName(); + try { + fileIndex.set(fileName, span.length, span.lastAccessTimestamp); + } catch (IOException e) { + throw new CacheException(e); + } } addSpan(span); - contentIndex.store(); + try { + contentIndex.store(); + } catch (IOException e) { + throw new CacheException(e); + } notifyAll(); } @@ -423,8 +456,14 @@ public final class SimpleCache implements Cache { public synchronized void applyContentMetadataMutations( String key, ContentMetadataMutations mutations) throws CacheException { Assertions.checkState(!released); + checkInitialization(); + contentIndex.applyContentMetadataMutations(key, mutations); - contentIndex.store(); + try { + contentIndex.store(); + } catch (IOException e) { + throw new CacheException(e); + } } @Override @@ -471,41 +510,46 @@ public final class SimpleCache implements Cache { /** Ensures that the cache's in-memory representation has been initialized. */ private void initialize() { if (!cacheDir.exists()) { - // Attempt to create the cache directory. if (!cacheDir.mkdirs()) { - // TODO: Initialization failed. Decide how to handle this. + initializationException = + new CacheException("Failed to create cache directory: " + cacheDir); return; } } File[] files = cacheDir.listFiles(); if (files == null) { - // TODO: Initialization failed. Decide how to handle this. + initializationException = + new CacheException("Failed to list cache directory files: " + cacheDir); return; } try { uid = loadUid(cacheDir, files); } catch (IOException e) { - // TODO: Initialization failed. Decide how to handle this. + initializationException = new CacheException("Failed to load cache UID: " + cacheDir); return; } - // TODO: Handle content index initialization failures. - contentIndex.initialize(uid); - if (fileIndex != null) { - // TODO: Handle file index initialization failures. - fileIndex.initialize(uid); - Map fileMetadata = fileIndex.getAll(); - loadDirectory(cacheDir, /* isRoot= */ true, files, fileMetadata); - fileIndex.removeAll(fileMetadata.keySet()); - } else { - loadDirectory(cacheDir, /* isRoot= */ true, files, /* fileMetadata= */ null); + try { + contentIndex.initialize(uid); + if (fileIndex != null) { + fileIndex.initialize(uid); + Map fileMetadata = fileIndex.getAll(); + loadDirectory(cacheDir, /* isRoot= */ true, files, fileMetadata); + fileIndex.removeAll(fileMetadata.keySet()); + } else { + loadDirectory(cacheDir, /* isRoot= */ true, files, /* fileMetadata= */ null); + } + } catch (IOException e) { + initializationException = new CacheException(e); + return; } + contentIndex.removeEmpty(); try { contentIndex.store(); - } catch (CacheException e) { + } catch (IOException e) { Log.e(TAG, "Storing index file failed", e); } } @@ -580,7 +624,14 @@ public final class SimpleCache implements Cache { } totalSpace -= span.length; if (fileIndex != null) { - fileIndex.remove(span.file.getName()); + String fileName = span.file.getName(); + try { + fileIndex.remove(fileName); + } catch (IOException e) { + // This will leave a stale entry in the file index. It will be removed next time the cache + // is initialized. + Log.w(TAG, "Failed to remove file index entry for: " + fileName); + } } contentIndex.maybeRemove(cachedContent.key); notifySpanRemoved(span); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/database/VersionTableTest.java b/library/core/src/test/java/com/google/android/exoplayer2/database/VersionTableTest.java index c2d902ebb7..977feaf44b 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/database/VersionTableTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/database/VersionTableTest.java @@ -43,20 +43,20 @@ public class VersionTableTest { } @Test - public void getVersion_unsetFeature_returnsVersionUnset() { + public void getVersion_unsetFeature_returnsVersionUnset() throws DatabaseIOException { int version = VersionTable.getVersion(database, FEATURE_1, INSTANCE_1); assertThat(version).isEqualTo(VersionTable.VERSION_UNSET); } @Test - public void getVersion_unsetVersion_returnsVersionUnset() { + public void getVersion_unsetVersion_returnsVersionUnset() throws DatabaseIOException { VersionTable.setVersion(database, FEATURE_1, INSTANCE_1, 1); int version = VersionTable.getVersion(database, FEATURE_1, INSTANCE_2); assertThat(version).isEqualTo(VersionTable.VERSION_UNSET); } @Test - public void getVersion_returnsSetVersion() { + public void getVersion_returnsSetVersion() throws DatabaseIOException { VersionTable.setVersion(database, FEATURE_1, INSTANCE_1, 1); assertThat(VersionTable.getVersion(database, FEATURE_1, INSTANCE_1)).isEqualTo(1); @@ -74,7 +74,7 @@ public class VersionTableTest { } @Test - public void removeVersion_removesSetVersion() { + public void removeVersion_removesSetVersion() throws DatabaseIOException { VersionTable.setVersion(database, FEATURE_1, INSTANCE_1, 1); VersionTable.setVersion(database, FEATURE_1, INSTANCE_2, 2); assertThat(VersionTable.getVersion(database, FEATURE_1, INSTANCE_1)).isEqualTo(1); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/offline/DefaultDownloadIndexTest.java b/library/core/src/test/java/com/google/android/exoplayer2/offline/DefaultDownloadIndexTest.java index 72ef80336b..1c5ac3e0f7 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/offline/DefaultDownloadIndexTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/offline/DefaultDownloadIndexTest.java @@ -19,6 +19,7 @@ import static com.google.android.exoplayer2.offline.DefaultDownloadIndex.INSTANC import static com.google.common.truth.Truth.assertThat; import android.database.sqlite.SQLiteDatabase; +import com.google.android.exoplayer2.database.DatabaseIOException; import com.google.android.exoplayer2.database.ExoDatabaseProvider; import com.google.android.exoplayer2.database.VersionTable; import org.junit.After; @@ -47,12 +48,13 @@ public class DefaultDownloadIndexTest { } @Test - public void getDownloadState_nonExistingId_returnsNull() { + public void getDownloadState_nonExistingId_returnsNull() throws DatabaseIOException { assertThat(downloadIndex.getDownloadState("non existing id")).isNull(); } @Test - public void addAndGetDownloadState_nonExistingId_returnsTheSameDownloadState() { + public void addAndGetDownloadState_nonExistingId_returnsTheSameDownloadState() + throws DatabaseIOException { String id = "id"; DownloadState downloadState = new DownloadStateBuilder(id).build(); @@ -63,7 +65,8 @@ public class DefaultDownloadIndexTest { } @Test - public void addAndGetDownloadState_existingId_returnsUpdatedDownloadState() { + public void addAndGetDownloadState_existingId_returnsUpdatedDownloadState() + throws DatabaseIOException { String id = "id"; DownloadStateBuilder downloadStateBuilder = new DownloadStateBuilder(id); downloadIndex.putDownloadState(downloadStateBuilder.build()); @@ -97,7 +100,8 @@ public class DefaultDownloadIndexTest { } @Test - public void releaseAndRecreateDownloadIndex_returnsTheSameDownloadState() { + public void releaseAndRecreateDownloadIndex_returnsTheSameDownloadState() + throws DatabaseIOException { String id = "id"; DownloadState downloadState = new DownloadStateBuilder(id).build(); downloadIndex.putDownloadState(downloadState); @@ -109,12 +113,13 @@ public class DefaultDownloadIndexTest { } @Test - public void removeDownloadState_nonExistingId_doesNotFail() { + public void removeDownloadState_nonExistingId_doesNotFail() throws DatabaseIOException { downloadIndex.removeDownloadState("non existing id"); } @Test - public void removeDownloadState_existingId_getDownloadStateReturnsNull() { + public void removeDownloadState_existingId_getDownloadStateReturnsNull() + throws DatabaseIOException { String id = "id"; DownloadState downloadState = new DownloadStateBuilder(id).build(); downloadIndex.putDownloadState(downloadState); @@ -125,12 +130,13 @@ public class DefaultDownloadIndexTest { } @Test - public void getDownloadStates_emptyDownloadIndex_returnsEmptyArray() { + public void getDownloadStates_emptyDownloadIndex_returnsEmptyArray() throws DatabaseIOException { assertThat(downloadIndex.getDownloadStates().getCount()).isEqualTo(0); } @Test - public void getDownloadStates_noState_returnsAllDownloadStatusSortedByStartTime() { + public void getDownloadStates_noState_returnsAllDownloadStatusSortedByStartTime() + throws DatabaseIOException { DownloadState downloadState1 = new DownloadStateBuilder("id1").setStartTimeMs(1).build(); downloadIndex.putDownloadState(downloadState1); DownloadState downloadState2 = new DownloadStateBuilder("id2").setStartTimeMs(0).build(); @@ -147,7 +153,8 @@ public class DefaultDownloadIndexTest { } @Test - public void getDownloadStates_withStates_returnsAllDownloadStatusWithTheSameStates() { + public void getDownloadStates_withStates_returnsAllDownloadStatusWithTheSameStates() + throws DatabaseIOException { DownloadState downloadState1 = new DownloadStateBuilder("id1") .setStartTimeMs(0) @@ -179,7 +186,7 @@ public class DefaultDownloadIndexTest { } @Test - public void putDownloadState_setsVersion() { + public void putDownloadState_setsVersion() throws DatabaseIOException { SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase(); assertThat( VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE, INSTANCE_UID)) @@ -193,7 +200,7 @@ public class DefaultDownloadIndexTest { } @Test - public void downloadIndex_versionDowngradeWipesData() { + public void downloadIndex_versionDowngradeWipesData() throws DatabaseIOException { DownloadState downloadState1 = new DownloadStateBuilder("id1").build(); downloadIndex.putDownloadState(downloadState1); DownloadStateCursor cursor = downloadIndex.getDownloadStates(); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadIndexUtilTest.java b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadIndexUtilTest.java index a2b560cecd..3372d41d32 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadIndexUtilTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/offline/DownloadIndexUtilTest.java @@ -22,6 +22,7 @@ import android.net.Uri; import com.google.android.exoplayer2.database.ExoDatabaseProvider; import com.google.android.exoplayer2.util.Util; import java.io.File; +import java.io.IOException; import java.util.Arrays; import java.util.List; import org.junit.After; @@ -53,7 +54,7 @@ public class DownloadIndexUtilTest { } @Test - public void addAction_nonExistingDownloadState_createsNewDownloadState() { + public void addAction_nonExistingDownloadState_createsNewDownloadState() throws IOException { byte[] data = new byte[] {1, 2, 3, 4}; DownloadAction action = DownloadAction.createDownloadAction( @@ -71,7 +72,7 @@ public class DownloadIndexUtilTest { } @Test - public void addAction_existingDownloadState_createsMergedDownloadState() { + public void addAction_existingDownloadState_createsMergedDownloadState() throws IOException { StreamKey streamKey1 = new StreamKey(/* periodIndex= */ 3, /* groupIndex= */ 4, /* trackIndex= */ 5); StreamKey streamKey2 = @@ -105,7 +106,7 @@ public class DownloadIndexUtilTest { } @Test - public void upgradeActionFile_createsDownloadStates() throws Exception { + public void upgradeActionFile_createsDownloadStates() throws IOException { ActionFile actionFile = new ActionFile(tempFile); StreamKey streamKey1 = new StreamKey(/* periodIndex= */ 3, /* groupIndex= */ 4, /* trackIndex= */ 5); @@ -138,7 +139,8 @@ public class DownloadIndexUtilTest { assertDownloadIndexContainsAction(action3, DownloadState.STATE_REMOVING); } - private void assertDownloadIndexContainsAction(DownloadAction action, int state) { + private void assertDownloadIndexContainsAction(DownloadAction action, int state) + throws IOException { DownloadState downloadState = downloadIndex.getDownloadState(action.id); assertThat(downloadState).isNotNull(); assertThat(downloadState.type).isEqualTo(action.type); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java index 2b128314fe..fea82fa302 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java @@ -21,7 +21,6 @@ import static com.google.common.truth.Truth.assertWithMessage; import android.net.Uri; import androidx.annotation.Nullable; import android.util.SparseArray; -import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.testutil.TestUtil; import com.google.android.exoplayer2.util.Util; import java.io.File; @@ -222,8 +221,8 @@ public class CachedContentIndexTest { @Test public void testLegacyEncryption() throws Exception { - byte[] key = "Bar12345Bar12345".getBytes(C.UTF8_NAME); // 128 bit key - byte[] key2 = "Foo12345Foo12345".getBytes(C.UTF8_NAME); // 128 bit key + byte[] key = Util.getUtf8Bytes("Bar12345Bar12345"); // 128 bit key + byte[] key2 = Util.getUtf8Bytes("Foo12345Foo12345"); // 128 bit key assertStoredAndLoadedEqual(newLegacyInstance(key), newLegacyInstance(key)); diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java index 6c3521d62e..9ac126b7c6 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/SimpleCacheTest.java @@ -21,7 +21,6 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; import static org.mockito.Mockito.doAnswer; -import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.testutil.TestUtil; import com.google.android.exoplayer2.upstream.cache.Cache.CacheException; import com.google.android.exoplayer2.util.Util; @@ -229,7 +228,7 @@ public class SimpleCacheTest { @Test public void testEncryptedIndex() throws Exception { - byte[] key = "Bar12345Bar12345".getBytes(C.UTF8_NAME); // 128 bit key + byte[] key = Util.getUtf8Bytes("Bar12345Bar12345"); // 128 bit key SimpleCache simpleCache = getEncryptedSimpleCache(key); // write data @@ -248,7 +247,7 @@ public class SimpleCacheTest { @Test public void testEncryptedIndexWrongKey() throws Exception { - byte[] key = "Bar12345Bar12345".getBytes(C.UTF8_NAME); // 128 bit key + byte[] key = Util.getUtf8Bytes("Bar12345Bar12345"); // 128 bit key SimpleCache simpleCache = getEncryptedSimpleCache(key); // write data @@ -258,7 +257,7 @@ public class SimpleCacheTest { simpleCache.release(); // Reload cache - byte[] key2 = "Foo12345Foo12345".getBytes(C.UTF8_NAME); // 128 bit key + byte[] key2 = Util.getUtf8Bytes("Foo12345Foo12345"); // 128 bit key simpleCache = getEncryptedSimpleCache(key2); // Cache should be cleared @@ -268,7 +267,7 @@ public class SimpleCacheTest { @Test public void testEncryptedIndexLostKey() throws Exception { - byte[] key = "Bar12345Bar12345".getBytes(C.UTF8_NAME); // 128 bit key + byte[] key = Util.getUtf8Bytes("Bar12345Bar12345"); // 128 bit key SimpleCache simpleCache = getEncryptedSimpleCache(key); // write data