CacheUtil: Remove confusing remove(DataSpec) method

- The remove(DataSpec) method was confusing because it ignored
  the DataSpec position and range, and instead removed all data
  with a matching cache key.
- The remove(String) method seems better put directly on the
  Cache interface.

PiperOrigin-RevId: 312113302
This commit is contained in:
olly 2020-05-18 18:59:39 +01:00 committed by Oliver Woodman
parent 377bf27f47
commit cccb9e1ae8
9 changed files with 38 additions and 79 deletions

View File

@ -134,7 +134,7 @@ public final class ProgressiveDownloader implements Downloader {
@Override
public void remove() {
CacheUtil.remove(dataSpec, dataSource.getCache(), dataSource.getCacheKeyFactory());
dataSource.getCache().removeSpans(dataSource.getCacheKeyFactory().buildCacheKey(dataSpec));
}
private static final class ProgressForwarder implements CacheUtil.ProgressListener {

View File

@ -205,13 +205,13 @@ public abstract class SegmentDownloader<M extends FilterableManifest<M>> impleme
M manifest = getManifest(dataSource, manifestDataSpec);
List<Segment> segments = getSegments(dataSource, manifest, true);
for (int i = 0; i < segments.size(); i++) {
CacheUtil.remove(segments.get(i).dataSpec, cache, cacheKeyFactory);
cache.removeSpans(cacheKeyFactory.buildCacheKey(segments.get(i).dataSpec));
}
} catch (IOException e) {
// Ignore exceptions when removing.
} finally {
// Always attempt to remove the manifest.
CacheUtil.remove(manifestDataSpec, cache, cacheKeyFactory);
cache.removeSpans(cacheKeyFactory.buildCacheKey(manifestDataSpec));
}
}

View File

@ -229,16 +229,23 @@ public interface Cache {
*/
void releaseHoleSpan(CacheSpan holeSpan);
/**
* Removes all {@link CacheSpan CacheSpans} with the given key, deleting the underlying files.
*
* @param key The cache key for the data.
*/
@WorkerThread
void removeSpans(String key);
/**
* Removes a cached {@link CacheSpan} from the cache, deleting the underlying file.
*
* <p>This method may be slow and shouldn't normally be called on the main thread.
*
* @param span The {@link CacheSpan} to remove.
* @throws CacheException If an error is encountered.
*/
@WorkerThread
void removeSpan(CacheSpan span) throws CacheException;
void removeSpan(CacheSpan span);
/**
* Queries if a range is entirely available in the cache.

View File

@ -28,7 +28,6 @@ import com.google.android.exoplayer2.util.Util;
import java.io.EOFException;
import java.io.IOException;
import java.io.InterruptedIOException;
import java.util.NavigableSet;
import java.util.concurrent.atomic.AtomicBoolean;
/**
@ -314,41 +313,6 @@ public final class CacheUtil {
}
}
/**
* Removes all of the data specified by the {@code dataSpec}.
*
* <p>This methods blocks until the operation is complete.
*
* @param dataSpec Defines the data to be removed.
* @param cache A {@link Cache} to store the data.
* @param cacheKeyFactory An optional factory for cache keys.
*/
@WorkerThread
public static void remove(
DataSpec dataSpec, Cache cache, @Nullable CacheKeyFactory cacheKeyFactory) {
remove(cache, buildCacheKey(dataSpec, cacheKeyFactory));
}
/**
* Removes all of the data specified by the {@code key}.
*
* <p>This methods blocks until the operation is complete.
*
* @param cache A {@link Cache} to store the data.
* @param key The key whose data should be removed.
*/
@WorkerThread
public static void remove(Cache cache, String key) {
NavigableSet<CacheSpan> cachedSpans = cache.getCachedSpans(key);
for (CacheSpan cachedSpan : cachedSpans) {
try {
cache.removeSpan(cachedSpan);
} catch (Cache.CacheException e) {
// Do nothing.
}
}
}
private static String buildCacheKey(
DataSpec dataSpec, @Nullable CacheKeyFactory cacheKeyFactory) {
return (cacheKeyFactory != null ? cacheKeyFactory : CacheKeyFactory.DEFAULT)

View File

@ -16,7 +16,6 @@
package com.google.android.exoplayer2.upstream.cache;
import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.upstream.cache.Cache.CacheException;
import java.util.TreeSet;
/** Evicts least recently used cache files first. */
@ -70,11 +69,7 @@ public final class LeastRecentlyUsedCacheEvictor implements CacheEvictor {
private void evictCache(Cache cache, long requiredSpace) {
while (currentSize + requiredSpace > maxBytes && !leastRecentlyUsed.isEmpty()) {
try {
cache.removeSpan(leastRecentlyUsed.first());
} catch (CacheException e) {
// do nothing.
}
cache.removeSpan(leastRecentlyUsed.first());
}
}

View File

@ -470,6 +470,14 @@ public final class SimpleCache implements Cache {
notifyAll();
}
@Override
public synchronized void removeSpans(String key) {
Assertions.checkState(!released);
for (CacheSpan span : getCachedSpans(key)) {
removeSpanInternal(span);
}
}
@Override
public synchronized void removeSpan(CacheSpan span) {
Assertions.checkState(!released);

View File

@ -432,7 +432,7 @@ public final class CacheDataSourceTest {
TestUtil.readExactly(cacheDataSource, 100);
// Delete cached data.
CacheUtil.remove(unboundedDataSpec, cache, /* cacheKeyFactory= */ null);
cache.removeSpans(cacheDataSource.getCacheKeyFactory().buildCacheKey(unboundedDataSpec));
assertCacheEmpty(cache);
// Read the rest of the data.

View File

@ -15,7 +15,6 @@
*/
package com.google.android.exoplayer2.upstream.cache;
import static com.google.android.exoplayer2.testutil.CacheAsserts.assertCacheEmpty;
import static com.google.android.exoplayer2.testutil.CacheAsserts.assertCachedData;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.fail;
@ -29,7 +28,6 @@ import com.google.android.exoplayer2.testutil.FakeDataSet;
import com.google.android.exoplayer2.testutil.FakeDataSource;
import com.google.android.exoplayer2.testutil.TestUtil;
import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.FileDataSource;
import com.google.android.exoplayer2.util.Util;
import java.io.EOFException;
import java.io.File;
@ -287,35 +285,6 @@ public final class CacheUtilTest {
assertCachedData(cache, fakeDataSet);
}
@Test
public void remove() throws Exception {
FakeDataSet fakeDataSet = new FakeDataSet().setRandomData("test_data", 100);
FakeDataSource dataSource = new FakeDataSource(fakeDataSet);
DataSpec dataSpec =
new DataSpec.Builder()
.setUri("test_data")
.setFlags(DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION)
.build();
CacheUtil.cache(
// Set fragmentSize to 10 to make sure there are multiple spans.
new CacheDataSource(
cache,
dataSource,
new FileDataSource(),
new CacheDataSink(cache, /* fragmentSize= */ 10),
/* flags= */ 0,
/* eventListener= */ null),
dataSpec,
/* progressListener= */ null,
/* isCanceled= */ null,
/* enableEOFException= */ true,
/* temporaryBuffer= */ new byte[CacheUtil.DEFAULT_BUFFER_SIZE_BYTES]);
CacheUtil.remove(dataSpec, cache, /* cacheKeyFactory= */ null);
assertCacheEmpty(cache);
}
private static final class CachingCounters implements CacheUtil.ProgressListener {
private long contentLength = C.LENGTH_UNSET;

View File

@ -225,6 +225,22 @@ public class SimpleCacheTest {
Util.recursiveDelete(cacheDir2);
}
@Test
public void removeSpans_removesSpansWithSameKey() throws Exception {
SimpleCache simpleCache = getSimpleCache();
CacheSpan holeSpan = simpleCache.startReadWrite(KEY_1, 0);
addCache(simpleCache, KEY_1, 0, 10);
addCache(simpleCache, KEY_1, 20, 10);
simpleCache.releaseHoleSpan(holeSpan);
holeSpan = simpleCache.startReadWrite(KEY_2, 20);
addCache(simpleCache, KEY_2, 20, 10);
simpleCache.releaseHoleSpan(holeSpan);
simpleCache.removeSpans(KEY_1);
assertThat(simpleCache.getCachedSpans(KEY_1)).isEmpty();
assertThat(simpleCache.getCachedSpans(KEY_2)).hasSize(1);
}
@Test
public void encryptedIndex() throws Exception {
byte[] key = Util.getUtf8Bytes("Bar12345Bar12345"); // 128 bit key