Commit 3e03e71c by olly Committed by Oliver Woodman

Start enabling database based SimpleCache indexing

- Expose constructor (package private for now, for tests only)
- Add some tests for cache initialization
- Add some TODOs for handling initialization failure

PiperOrigin-RevId: 235188386
parent ff7f0304
...@@ -126,7 +126,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; ...@@ -126,7 +126,7 @@ import org.checkerframework.checker.nullness.compatqual.NullableType;
* @param legacyStorageSecretKey A 16 byte AES key for reading, and optionally writing, legacy * @param legacyStorageSecretKey A 16 byte AES key for reading, and optionally writing, legacy
* storage. * storage.
* @param legacyStorageEncrypt Whether to encrypt when writing to legacy storage. Must be false if * @param legacyStorageEncrypt Whether to encrypt when writing to legacy storage. Must be false if
* {@code secretKey} is null. * {@code legacyStorageSecretKey} is null.
* @param preferLegacyStorage Whether to use prefer legacy storage if both storage types are * @param preferLegacyStorage Whether to use prefer legacy storage if both storage types are
* enabled. This option is only useful for downgrading from database storage back to legacy * enabled. This option is only useful for downgrading from database storage back to legacy
* storage. * storage.
......
...@@ -19,6 +19,7 @@ import android.os.ConditionVariable; ...@@ -19,6 +19,7 @@ import android.os.ConditionVariable;
import androidx.annotation.NonNull; import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.database.DatabaseProvider;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Log; import com.google.android.exoplayer2.util.Log;
import java.io.File; import java.io.File;
...@@ -61,6 +62,7 @@ public final class SimpleCache implements Cache { ...@@ -61,6 +62,7 @@ public final class SimpleCache implements Cache {
private final HashMap<String, ArrayList<Listener>> listeners; private final HashMap<String, ArrayList<Listener>> listeners;
private final Random random; private final Random random;
private long uid;
private long totalSpace; private long totalSpace;
private boolean released; private boolean released;
...@@ -109,7 +111,7 @@ public final class SimpleCache implements Cache { ...@@ -109,7 +111,7 @@ public final class SimpleCache implements Cache {
* @param secretKey If not null, cache keys will be stored encrypted on filesystem using AES/CBC. * @param secretKey If not null, cache keys will be stored encrypted on filesystem using AES/CBC.
* The key must be 16 bytes long. * The key must be 16 bytes long.
*/ */
public SimpleCache(File cacheDir, CacheEvictor evictor, byte[] secretKey) { public SimpleCache(File cacheDir, CacheEvictor evictor, @Nullable byte[] secretKey) {
this(cacheDir, evictor, secretKey, secretKey != null); this(cacheDir, evictor, secretKey, secretKey != null);
} }
...@@ -124,16 +126,55 @@ public final class SimpleCache implements Cache { ...@@ -124,16 +126,55 @@ public final class SimpleCache implements Cache {
* @param encrypt Whether the index will be encrypted when written. Must be false if {@code * @param encrypt Whether the index will be encrypted when written. Must be false if {@code
* secretKey} is null. * secretKey} is null.
*/ */
public SimpleCache(File cacheDir, CacheEvictor evictor, byte[] secretKey, boolean encrypt) { public SimpleCache(
File cacheDir, CacheEvictor evictor, @Nullable byte[] secretKey, boolean encrypt) {
this( this(
cacheDir, cacheDir,
evictor, evictor,
new CachedContentIndex(
/* databaseProvider= */ null, /* databaseProvider= */ null,
cacheDir,
secretKey, secretKey,
encrypt, encrypt,
/* preferLegacyStorage= */ true)); /* preferLegacyIndex= */ true);
}
// TODO: Make public and consider a constructor that takes DatabaseProvider but not legacy args.
/**
* Constructs the cache. The cache will delete any unrecognized files from the cache directory.
* Hence the directory cannot be used to store other files.
*
* @param cacheDir A dedicated cache directory.
* @param evictor The evictor to be used.
* @param databaseProvider Provides the database in which the cache index is stored, or {@code
* null} to use a legacy index. Using the database index is highly recommended for performance
* reasons.
* @param legacyIndexSecretKey A 16 byte AES key for reading, and optionally writing, the legacy
* index. Not used by the database index, however should still be provided when using the
* database index in cases where upgrading from the legacy index may be necessary.
* @param legacyIndexEncrypt Whether to encrypt when writing to the legacy index. Must be false if
* {@code legacyIndexSecretKey} is null. Not used by the database index.
* @param preferLegacyIndex Whether to use the legacy index even if a {@code databaseProvider} is
* provided. Should be {@code false} in most cases. Setting this to {@code true} is only
* useful for downgrading from the database index back to the legacy index.
*/
/* package */ SimpleCache(
File cacheDir,
CacheEvictor evictor,
@Nullable DatabaseProvider databaseProvider,
@Nullable byte[] legacyIndexSecretKey,
boolean legacyIndexEncrypt,
boolean preferLegacyIndex) {
this(
cacheDir,
evictor,
new CachedContentIndex(
databaseProvider,
cacheDir,
legacyIndexSecretKey,
legacyIndexEncrypt,
preferLegacyIndex),
databaseProvider != null && !preferLegacyIndex
? new CacheFileMetadataIndex(databaseProvider)
: null);
} }
/** /**
...@@ -143,8 +184,13 @@ public final class SimpleCache implements Cache { ...@@ -143,8 +184,13 @@ public final class SimpleCache implements Cache {
* @param cacheDir A dedicated cache directory. * @param cacheDir A dedicated cache directory.
* @param evictor The evictor to be used. * @param evictor The evictor to be used.
* @param contentIndex The content index to be used. * @param contentIndex The content index to be used.
* @param fileIndex The file index to be used.
*/ */
/* package */ SimpleCache(File cacheDir, CacheEvictor evictor, CachedContentIndex contentIndex) { /* package */ SimpleCache(
File cacheDir,
CacheEvictor evictor,
CachedContentIndex contentIndex,
@Nullable CacheFileMetadataIndex fileIndex) {
if (!lockFolder(cacheDir)) { if (!lockFolder(cacheDir)) {
throw new IllegalStateException("Another SimpleCache instance uses the folder: " + cacheDir); throw new IllegalStateException("Another SimpleCache instance uses the folder: " + cacheDir);
} }
...@@ -152,9 +198,10 @@ public final class SimpleCache implements Cache { ...@@ -152,9 +198,10 @@ public final class SimpleCache implements Cache {
this.cacheDir = cacheDir; this.cacheDir = cacheDir;
this.evictor = evictor; this.evictor = evictor;
this.contentIndex = contentIndex; this.contentIndex = contentIndex;
this.fileIndex = null; this.fileIndex = fileIndex;
listeners = new HashMap<>(); listeners = new HashMap<>();
random = new Random(); random = new Random();
uid = -1;
// Start cache initialization. // Start cache initialization.
final ConditionVariable conditionVariable = new ConditionVariable(); final ConditionVariable conditionVariable = new ConditionVariable();
...@@ -261,7 +308,7 @@ public final class SimpleCache implements Cache { ...@@ -261,7 +308,7 @@ public final class SimpleCache implements Cache {
// Read case. // Read case.
if (span.isCached) { if (span.isCached) {
String fileName = span.file.getName(); String fileName = Assertions.checkNotNull(span.file).getName();
long length = span.length; long length = span.length;
long lastAccessTimestamp = System.currentTimeMillis(); long lastAccessTimestamp = System.currentTimeMillis();
boolean updateFile = false; boolean updateFile = false;
...@@ -386,6 +433,11 @@ public final class SimpleCache implements Cache { ...@@ -386,6 +433,11 @@ public final class SimpleCache implements Cache {
return contentIndex.getContentMetadata(key); return contentIndex.getContentMetadata(key);
} }
/** Returns the non-negative cache UID, or -1 if cache initialization failed. */
/* package */ synchronized long getUid() {
return uid;
}
/** /**
* Returns the cache {@link SimpleCacheSpan} corresponding to the provided lookup {@link * Returns the cache {@link SimpleCacheSpan} corresponding to the provided lookup {@link
* SimpleCacheSpan}. * SimpleCacheSpan}.
...@@ -419,21 +471,30 @@ public final class SimpleCache implements Cache { ...@@ -419,21 +471,30 @@ public final class SimpleCache implements Cache {
/** Ensures that the cache's in-memory representation has been initialized. */ /** Ensures that the cache's in-memory representation has been initialized. */
private void initialize() { private void initialize() {
if (!cacheDir.exists()) { if (!cacheDir.exists()) {
cacheDir.mkdirs(); // Attempt to create the cache directory.
if (!cacheDir.mkdirs()) {
// TODO: Initialization failed. Decide how to handle this.
return; return;
} }
}
File[] files = cacheDir.listFiles(); File[] files = cacheDir.listFiles();
if (files == null) {
// TODO: Initialization failed. Decide how to handle this.
return;
}
long uid = 0;
try { try {
uid = loadUid(cacheDir, files); uid = loadUid(cacheDir, files);
} catch (IOException e) { } catch (IOException e) {
// TODO: Decide how to handle this. // TODO: Initialization failed. Decide how to handle this.
return;
} }
// TODO: Handle content index initialization failures.
contentIndex.initialize(uid); contentIndex.initialize(uid);
if (fileIndex != null) { if (fileIndex != null) {
// TODO: Handle file index initialization failures.
fileIndex.initialize(uid); fileIndex.initialize(uid);
Map<String, CacheFileMetadata> fileMetadata = fileIndex.getAll(); Map<String, CacheFileMetadata> fileMetadata = fileIndex.getAll();
loadDirectory(cacheDir, /* isRoot= */ true, files, fileMetadata); loadDirectory(cacheDir, /* isRoot= */ true, files, fileMetadata);
...@@ -462,7 +523,7 @@ public final class SimpleCache implements Cache { ...@@ -462,7 +523,7 @@ public final class SimpleCache implements Cache {
private void loadDirectory( private void loadDirectory(
File directory, File directory,
boolean isRoot, boolean isRoot,
File[] files, @Nullable File[] files,
@Nullable Map<String, CacheFileMetadata> fileMetadata) { @Nullable Map<String, CacheFileMetadata> fileMetadata) {
if (files == null || files.length == 0) { if (files == null || files.length == 0) {
// Either (a) directory isn't really a directory (b) it's empty, or (c) listing files failed. // Either (a) directory isn't really a directory (b) it's empty, or (c) listing files failed.
...@@ -582,7 +643,6 @@ public final class SimpleCache implements Cache { ...@@ -582,7 +643,6 @@ public final class SimpleCache implements Cache {
* @throws IOException If there is an error loading or generating the UID. * @throws IOException If there is an error loading or generating the UID.
*/ */
private static long loadUid(File directory, File[] files) throws IOException { private static long loadUid(File directory, File[] files) throws IOException {
if (files != null) {
for (File file : files) { for (File file : files) {
String fileName = file.getName(); String fileName = file.getName();
if (fileName.endsWith(UID_FILE_SUFFIX)) { if (fileName.endsWith(UID_FILE_SUFFIX)) {
...@@ -595,7 +655,6 @@ public final class SimpleCache implements Cache { ...@@ -595,7 +655,6 @@ public final class SimpleCache implements Cache {
} }
} }
} }
}
return createUid(directory); return createUid(directory);
} }
......
...@@ -55,15 +55,44 @@ public class SimpleCacheTest { ...@@ -55,15 +55,44 @@ public class SimpleCacheTest {
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
MockitoAnnotations.initMocks(this); MockitoAnnotations.initMocks(this);
cacheDir = Util.createTempDirectory(RuntimeEnvironment.application, "ExoPlayerTest"); cacheDir = Util.createTempFile(RuntimeEnvironment.application, "ExoPlayerTest");
// Delete the file. SimpleCache initialization should create a directory with the same name.
assertThat(cacheDir.delete()).isTrue();
} }
@After @After
public void tearDown() throws Exception { public void tearDown() {
Util.recursiveDelete(cacheDir); Util.recursiveDelete(cacheDir);
} }
@Test @Test
public void testCacheInitialization() {
SimpleCache cache = getSimpleCache();
// Cache initialization should have created a non-negative UID.
long uid = cache.getUid();
assertThat(uid).isAtLeast(0L);
// And the cache directory.
assertThat(cacheDir.exists()).isTrue();
// Reinitialization should load the same non-negative UID.
cache.release();
cache = getSimpleCache();
assertThat(cache.getUid()).isEqualTo(uid);
}
@Test
public void testCacheInitializationError() throws IOException {
// Creating a file where the cache should be will cause an error during initialization.
assertThat(cacheDir.createNewFile()).isTrue();
// Cache initialization should not throw an exception, but no UID will be generated.
SimpleCache cache = getSimpleCache();
long uid = cache.getUid();
assertThat(uid).isEqualTo(-1L);
}
@Test
public void testCommittingOneFile() throws Exception { public void testCommittingOneFile() throws Exception {
SimpleCache simpleCache = getSimpleCache(); SimpleCache simpleCache = getSimpleCache();
...@@ -294,10 +323,11 @@ public class SimpleCacheTest { ...@@ -294,10 +323,11 @@ public class SimpleCacheTest {
/* Tests https://github.com/google/ExoPlayer/issues/3260 case. */ /* Tests https://github.com/google/ExoPlayer/issues/3260 case. */
@Test @Test
public void testExceptionDuringEvictionByLeastRecentlyUsedCacheEvictorNotHang() throws Exception { public void testExceptionDuringEvictionByLeastRecentlyUsedCacheEvictorNotHang() throws Exception {
CachedContentIndex index = CachedContentIndex contentIndex =
Mockito.spy(new CachedContentIndex(TestUtil.getTestDatabaseProvider())); Mockito.spy(new CachedContentIndex(TestUtil.getTestDatabaseProvider()));
SimpleCache simpleCache = SimpleCache simpleCache =
new SimpleCache(cacheDir, new LeastRecentlyUsedCacheEvictor(20), index); new SimpleCache(
cacheDir, new LeastRecentlyUsedCacheEvictor(20), contentIndex, /* fileIndex= */ null);
// Add some content. // Add some content.
CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, 0); CacheSpan cacheSpan = simpleCache.startReadWrite(KEY_1, 0);
...@@ -308,7 +338,7 @@ public class SimpleCacheTest { ...@@ -308,7 +338,7 @@ public class SimpleCacheTest {
invocation -> { invocation -> {
throw new CacheException("SimpleCacheTest"); throw new CacheException("SimpleCacheTest");
}) })
.when(index) .when(contentIndex)
.store(); .store();
// Adding more content will make LeastRecentlyUsedCacheEvictor evict previous content. // Adding more content will make LeastRecentlyUsedCacheEvictor evict previous content.
......
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