Skip to content

SDK / exoplayer

Go to a project
Commit 39505452 by olly Committed by Oliver Woodman
Options

Make VersionTable static 

The way it is currently, it's very unclear that an operation on the
version table will correctly belong to a transaction in code such as
this, taken from DefaultDownloadIndex:

writableDatabase.beginTransaction();
try {
  writableDatabase.execSQL(...);
  versionTable.setVersion(...);
  writableDatabase.setTransactionSuccessful();
} finally {
  writableDatabase.endTransaction();
}

This change explicitly passes the database, to make it obvious that
the operation will really go into the same transaction:

writableDatabase.beginTransaction();
try {
  writableDatabase.execSQL(....);
  VersionTable.setVersion(writableDatabase, ...);
  writableDatabase.setTransactionSuccessful();
} finally {
  writableDatabase.endTransaction();
}

PiperOrigin-RevId: 231374933
parent bac8dfea
Inline Side-by-side
Showing with 45 additions and 42 deletions
library/core/src/main/java/com/google/android/exoplayer2/database/VersionTable.java
...@@ -20,17 +20,18 @@ import android.database.Cursor; ...@@ -20,17 +20,18 @@ import android.database.Cursor;
import android.database.DatabaseUtils; import android.database.DatabaseUtils;
import android.database.sqlite.SQLiteDatabase; import android.database.sqlite.SQLiteDatabase;
import android.support.annotation.IntDef; import android.support.annotation.IntDef;
import android.support.annotation.VisibleForTesting;
import java.lang.annotation.Documented; import java.lang.annotation.Documented;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
/** /**
* A table that holds version information about other ExoPlayer tables. This allows ExoPlayer tables * Utility methods for accessing versions of ExoPlayer database components. This allows them to be
* to be versioned independently to the version of the containing database. * versioned independently to the version of the containing database.
*/ */
public final class VersionTable { public final class VersionTable {
/** Returned by {@link #getVersion(int)} if the version is unset. */ /** Returned by {@link #getVersion(SQLiteDatabase, int)} if the version is unset. */
public static final int VERSION_UNSET = -1; public static final int VERSION_UNSET = -1;
/** Version of tables used for offline functionality. */ /** Version of tables used for offline functionality. */
public static final int FEATURE_OFFLINE = 0; public static final int FEATURE_OFFLINE = 0;
...@@ -56,41 +57,39 @@ public final class VersionTable { ...@@ -56,41 +57,39 @@ public final class VersionTable {
@IntDef({FEATURE_OFFLINE, FEATURE_CACHE}) @IntDef({FEATURE_OFFLINE, FEATURE_CACHE})
private @interface Feature {} private @interface Feature {}
private final DatabaseProvider databaseProvider; private VersionTable() {}
public VersionTable(DatabaseProvider databaseProvider) {
this.databaseProvider = databaseProvider;
// Check whether the table exists to avoid getting a writable database if we don't need one.
if (!doesTableExist(databaseProvider, TABLE_NAME)) {
databaseProvider.getWritableDatabase().execSQL(SQL_CREATE_TABLE_IF_NOT_EXISTS);
}
}
/** /**
* Sets the version of tables belonging to the specified feature. * Sets the version of tables belonging to the specified feature.
* *
* @param writableDatabase The database to update.
* @param feature The feature. * @param feature The feature.
* @param version The version. * @param version The version.
*/ */
public void setVersion(@Feature int feature, int version) { public static void setVersion(
SQLiteDatabase writableDatabase, @Feature int feature, int version) {
writableDatabase.execSQL(SQL_CREATE_TABLE_IF_NOT_EXISTS);
ContentValues values = new ContentValues(); ContentValues values = new ContentValues();
values.put(COLUMN_FEATURE, feature); values.put(COLUMN_FEATURE, feature);
values.put(COLUMN_VERSION, version); values.put(COLUMN_VERSION, version);
SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
writableDatabase.replace(TABLE_NAME, /* nullColumnHack= */ null, values); writableDatabase.replace(TABLE_NAME, /* nullColumnHack= */ null, values);
} }
/** /**
* Returns the version of tables belonging to the specified feature, or {@link #VERSION_UNSET} if * Returns the version of tables belonging to the specified feature, or {@link #VERSION_UNSET} if
* no version information is available. * no version information is available.
*
* @param database The database to query.
* @param feature The feature.
*/ */
public int getVersion(@Feature int feature) { public static int getVersion(SQLiteDatabase database, @Feature int feature) {
if (!tableExists(database, TABLE_NAME)) {
return VERSION_UNSET;
}
String selection = COLUMN_FEATURE + " = ?"; String selection = COLUMN_FEATURE + " = ?";
String[] selectionArgs = {Integer.toString(feature)}; String[] selectionArgs = {Integer.toString(feature)};
try (Cursor cursor = try (Cursor cursor =
databaseProvider database.query(
.getReadableDatabase()
.query(
TABLE_NAME, TABLE_NAME,
new String[] {COLUMN_VERSION}, new String[] {COLUMN_VERSION},
selection, selection,
...@@ -106,8 +105,8 @@ public final class VersionTable { ...@@ -106,8 +105,8 @@ public final class VersionTable {
} }
} }
/* package */ static boolean doesTableExist(DatabaseProvider databaseProvider, String tableName) { @VisibleForTesting
SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase(); /* package */ static boolean tableExists(SQLiteDatabase readableDatabase, String tableName) {
long count = long count =
DatabaseUtils.queryNumEntries( DatabaseUtils.queryNumEntries(
readableDatabase, "sqlite_master", "tbl_name = ?", new String[] {tableName}); readableDatabase, "sqlite_master", "tbl_name = ?", new String[] {tableName});
......
library/core/src/main/java/com/google/android/exoplayer2/offline/DefaultDownloadIndex.java
...@@ -215,15 +215,16 @@ public final class DefaultDownloadIndex implements DownloadIndex { ...@@ -215,15 +215,16 @@ public final class DefaultDownloadIndex implements DownloadIndex {
public DownloadsTable(DatabaseProvider databaseProvider) { public DownloadsTable(DatabaseProvider databaseProvider) {
this.databaseProvider = databaseProvider; this.databaseProvider = databaseProvider;
VersionTable versionTable = new VersionTable(databaseProvider); int version =
int version = versionTable.getVersion(VersionTable.FEATURE_OFFLINE); VersionTable.getVersion(
databaseProvider.getReadableDatabase(), VersionTable.FEATURE_OFFLINE);
if (version == VersionTable.VERSION_UNSET || version > TABLE_VERSION) { if (version == VersionTable.VERSION_UNSET || version > TABLE_VERSION) {
SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase(); SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
writableDatabase.beginTransaction(); writableDatabase.beginTransaction();
try { try {
VersionTable.setVersion(writableDatabase, VersionTable.FEATURE_OFFLINE, TABLE_VERSION);
writableDatabase.execSQL(SQL_DROP_TABLE_IF_EXISTS); writableDatabase.execSQL(SQL_DROP_TABLE_IF_EXISTS);
writableDatabase.execSQL(SQL_CREATE_TABLE); writableDatabase.execSQL(SQL_CREATE_TABLE);
versionTable.setVersion(VersionTable.FEATURE_OFFLINE, TABLE_VERSION);
writableDatabase.setTransactionSuccessful(); writableDatabase.setTransactionSuccessful();
} finally { } finally {
writableDatabase.endTransaction(); writableDatabase.endTransaction();
......
library/core/src/test/java/com/google/android/exoplayer2/database/VersionTableTest.java
...@@ -19,6 +19,7 @@ import static com.google.android.exoplayer2.database.VersionTable.FEATURE_CACHE; ...@@ -19,6 +19,7 @@ import static com.google.android.exoplayer2.database.VersionTable.FEATURE_CACHE;
import static com.google.android.exoplayer2.database.VersionTable.FEATURE_OFFLINE; import static com.google.android.exoplayer2.database.VersionTable.FEATURE_OFFLINE;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import android.database.sqlite.SQLiteDatabase;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
...@@ -31,10 +32,14 @@ import org.robolectric.RuntimeEnvironment; ...@@ -31,10 +32,14 @@ import org.robolectric.RuntimeEnvironment;
public class VersionTableTest { public class VersionTableTest {
private ExoDatabaseProvider databaseProvider; private ExoDatabaseProvider databaseProvider;
private SQLiteDatabase readableDatabase;
private SQLiteDatabase writableDatabase;
@Before @Before
public void setUp() { public void setUp() {
databaseProvider = new ExoDatabaseProvider(RuntimeEnvironment.application); databaseProvider = new ExoDatabaseProvider(RuntimeEnvironment.application);
readableDatabase = databaseProvider.getReadableDatabase();
writableDatabase = databaseProvider.getWritableDatabase();
} }
@After @After
...@@ -44,35 +49,32 @@ public class VersionTableTest { ...@@ -44,35 +49,32 @@ public class VersionTableTest {
@Test @Test
public void getVersion_nonExistingTable_returnsVersionUnset() { public void getVersion_nonExistingTable_returnsVersionUnset() {
VersionTable versionTable = new VersionTable(databaseProvider); int version = VersionTable.getVersion(readableDatabase, FEATURE_OFFLINE);
int version = versionTable.getVersion(FEATURE_OFFLINE);
assertThat(version).isEqualTo(VersionTable.VERSION_UNSET); assertThat(version).isEqualTo(VersionTable.VERSION_UNSET);
} }
@Test @Test
public void getVersion_returnsSetVersion() { public void getVersion_returnsSetVersion() {
VersionTable versionTable = new VersionTable(databaseProvider); VersionTable.setVersion(writableDatabase, FEATURE_OFFLINE, 1);
assertThat(VersionTable.getVersion(readableDatabase, FEATURE_OFFLINE)).isEqualTo(1);
versionTable.setVersion(FEATURE_OFFLINE, 1); VersionTable.setVersion(writableDatabase, FEATURE_OFFLINE, 10);
assertThat(versionTable.getVersion(FEATURE_OFFLINE)).isEqualTo(1); assertThat(VersionTable.getVersion(readableDatabase, FEATURE_OFFLINE)).isEqualTo(10);
versionTable.setVersion(FEATURE_OFFLINE, 10); VersionTable.setVersion(writableDatabase, FEATURE_CACHE, 5);
assertThat(versionTable.getVersion(FEATURE_OFFLINE)).isEqualTo(10); assertThat(VersionTable.getVersion(readableDatabase, FEATURE_CACHE)).isEqualTo(5);
assertThat(VersionTable.getVersion(readableDatabase, FEATURE_OFFLINE)).isEqualTo(10);
versionTable.setVersion(FEATURE_CACHE, 5);
assertThat(versionTable.getVersion(FEATURE_CACHE)).isEqualTo(5);
assertThat(versionTable.getVersion(FEATURE_OFFLINE)).isEqualTo(10);
} }
@Test @Test
public void doesTableExist_nonExistingTable_returnsFalse() { public void doesTableExist_nonExistingTable_returnsFalse() {
assertThat(VersionTable.doesTableExist(databaseProvider, "NonExistingTable")).isFalse(); assertThat(VersionTable.tableExists(readableDatabase, "NonExistingTable")).isFalse();
} }
@Test @Test
public void doesTableExist_existingTable_returnsTrue() { public void doesTableExist_existingTable_returnsTrue() {
String table = "TestTable"; String table = "TestTable";
databaseProvider.getWritableDatabase().execSQL("CREATE TABLE " + table + " (dummy INTEGER)"); databaseProvider.getWritableDatabase().execSQL("CREATE TABLE " + table + " (dummy INTEGER)");
assertThat(VersionTable.doesTableExist(databaseProvider, table)).isTrue(); assertThat(VersionTable.tableExists(readableDatabase, table)).isTrue();
} }
} }
library/core/src/test/java/com/google/android/exoplayer2/offline/DefaultDownloadIndexTest.java
...@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.offline; ...@@ -17,6 +17,7 @@ package com.google.android.exoplayer2.offline;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import android.database.sqlite.SQLiteDatabase;
import android.net.Uri; import android.net.Uri;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
...@@ -180,13 +181,13 @@ public class DefaultDownloadIndexTest { ...@@ -180,13 +181,13 @@ public class DefaultDownloadIndexTest {
@Test @Test
public void putDownloadState_setsVersion() { public void putDownloadState_setsVersion() {
VersionTable versionTable = new VersionTable(databaseProvider); SQLiteDatabase readableDatabase = databaseProvider.getReadableDatabase();
assertThat(versionTable.getVersion(VersionTable.FEATURE_OFFLINE)) assertThat(VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE))
.isEqualTo(VersionTable.VERSION_UNSET); .isEqualTo(VersionTable.VERSION_UNSET);
downloadIndex.putDownloadState(new DownloadStateBuilder("id1").build()); downloadIndex.putDownloadState(new DownloadStateBuilder("id1").build());
assertThat(versionTable.getVersion(VersionTable.FEATURE_OFFLINE)) assertThat(VersionTable.getVersion(readableDatabase, VersionTable.FEATURE_OFFLINE))
.isEqualTo(DefaultDownloadIndex.TABLE_VERSION); .isEqualTo(DefaultDownloadIndex.TABLE_VERSION);
} }
...@@ -198,15 +199,15 @@ public class DefaultDownloadIndexTest { ...@@ -198,15 +199,15 @@ public class DefaultDownloadIndexTest {
assertThat(cursor.getCount()).isEqualTo(1); assertThat(cursor.getCount()).isEqualTo(1);
cursor.close(); cursor.close();
VersionTable versionTable = new VersionTable(databaseProvider); SQLiteDatabase writableDatabase = databaseProvider.getWritableDatabase();
versionTable.setVersion(VersionTable.FEATURE_OFFLINE, Integer.MAX_VALUE); VersionTable.setVersion(writableDatabase, VersionTable.FEATURE_OFFLINE, Integer.MAX_VALUE);
downloadIndex = new DefaultDownloadIndex(databaseProvider); downloadIndex = new DefaultDownloadIndex(databaseProvider);
cursor = downloadIndex.getDownloadStates(); cursor = downloadIndex.getDownloadStates();
assertThat(cursor.getCount()).isEqualTo(0); assertThat(cursor.getCount()).isEqualTo(0);
cursor.close(); cursor.close();
assertThat(versionTable.getVersion(VersionTable.FEATURE_OFFLINE)) assertThat(VersionTable.getVersion(writableDatabase, VersionTable.FEATURE_OFFLINE))
.isEqualTo(DefaultDownloadIndex.TABLE_VERSION); .isEqualTo(DefaultDownloadIndex.TABLE_VERSION);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment