From 5925f92482e8352c6ff892b3dd292815df0d0982 Mon Sep 17 00:00:00 2001 From: eguven Date: Tue, 27 Mar 2018 04:30:11 -0700 Subject: [PATCH] Persist content redirected URI in CacheDataSource Issue: #2360 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=190597773 --- RELEASENOTES.md | 2 + .../cache/CachedContentIndexTest.java | 11 ++- .../upstream/cache/CacheDataSource.java | 75 ++++++++++++++++--- .../upstream/cache/CachedContent.java | 41 ++++++++-- .../upstream/cache/CachedContentIndex.java | 4 +- .../upstream/cache/ContentMetadata.java | 14 +++- .../cache/DefaultContentMetadata.java | 26 ++++--- .../upstream/cache/SimpleCache.java | 23 +++--- .../cache/DefaultContentMetadataTest.java | 18 ++--- 9 files changed, 159 insertions(+), 55 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 302e654a48..33de950ff6 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -38,6 +38,8 @@ * Add release method to Cache interface. * Prevent multiple instances of SimpleCache in the same folder. Previous instance must be released. + * Store redirected URI + ([#2360](https://github.com/google/ExoPlayer/issues/2360)). * DRM: * Allow multiple listeners for `DefaultDrmSessionManager`. * Pass `DrmSessionManager` to `ExoPlayerFactory` instead of `RendererFactory`. diff --git a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java index 4b52a15144..b6c67d32a1 100644 --- a/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java +++ b/library/core/src/androidTest/java/com/google/android/exoplayer2/upstream/cache/CachedContentIndexTest.java @@ -21,11 +21,11 @@ public class CachedContentIndexTest extends InstrumentationTestCase { 0, 0, 0, 1, // version 0, 0, 0, 0, // flags 0, 0, 0, 2, // number_of_CachedContent - 0, 0, 0, 5, // cache_id - 0, 5, 65, 66, 67, 68, 69, // cache_key + 0, 0, 0, 5, // cache_id 5 + 0, 5, 65, 66, 67, 68, 69, // cache_key "ABCDE" 0, 0, 0, 0, 0, 0, 0, 10, // original_content_length - 0, 0, 0, 2, // cache_id - 0, 5, 75, 76, 77, 78, 79, // cache_key + 0, 0, 0, 2, // cache_id 2 + 0, 5, 75, 76, 77, 78, 79, // cache_key "KLMNO" 0, 0, 0, 0, 0, 0, 10, 0, // original_content_length (byte) 0xF6, (byte) 0xFB, 0x50, 0x41 // hashcode_of_CachedContent_array }; @@ -285,8 +285,7 @@ public class CachedContentIndexTest extends InstrumentationTestCase { Set keys2 = index2.getKeys(); assertThat(keys2).isEqualTo(keys); for (String key : keys) { - assertThat(index2.getContentLength(key)).isEqualTo(index.getContentLength(key)); - assertThat(index2.get(key).getSpans()).isEqualTo(index.get(key).getSpans()); + assertThat(index2.get(key)).isEqualTo(index.get(key)); } } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java index da5251de08..53898a6c87 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java @@ -18,6 +18,7 @@ package com.google.android.exoplayer2.upstream.cache; import android.net.Uri; import android.support.annotation.IntDef; import android.support.annotation.Nullable; +import android.util.Log; import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.upstream.DataSink; import com.google.android.exoplayer2.upstream.DataSource; @@ -51,6 +52,8 @@ public final class CacheDataSource implements DataSource { */ public static final long DEFAULT_MAX_CACHE_FILE_SIZE = 2 * 1024 * 1024; + private static final String TAG = "CacheDataSource"; + /** * Flags controlling the cache's behavior. */ @@ -125,6 +128,7 @@ public final class CacheDataSource implements DataSource { private DataSource currentDataSource; private boolean currentDataSpecLengthUnset; private Uri uri; + private Uri actualUri; private int flags; private String key; private long readPosition; @@ -212,9 +216,10 @@ public final class CacheDataSource implements DataSource { @Override public long open(DataSpec dataSpec) throws IOException { try { - uri = dataSpec.uri; - flags = dataSpec.flags; key = CacheUtil.getKey(dataSpec); + uri = dataSpec.uri; + actualUri = loadRedirectedUriOrReturnGivenUri(cache, key, uri); + flags = dataSpec.flags; readPosition = dataSpec.position; currentRequestIgnoresCache = (ignoreCacheOnError && seenCacheError) || (dataSpec.length == C.LENGTH_UNSET && ignoreCacheForUnsetLengthRequests); @@ -251,7 +256,7 @@ public final class CacheDataSource implements DataSource { } int bytesRead = currentDataSource.read(buffer, offset, readLength); if (bytesRead != C.RESULT_END_OF_INPUT) { - if (currentDataSource == cacheReadDataSource) { + if (isReadingFromCache()) { totalCachedBytesRead += bytesRead; } readPosition += bytesRead; @@ -259,7 +264,7 @@ public final class CacheDataSource implements DataSource { bytesRemaining -= bytesRead; } } else if (currentDataSpecLengthUnset) { - setBytesRemaining(0); + setBytesRemainingAndMaybeStoreLength(0); } else if (bytesRemaining > 0 || bytesRemaining == C.LENGTH_UNSET) { closeCurrentSource(); openNextSource(false); @@ -268,7 +273,7 @@ public final class CacheDataSource implements DataSource { return bytesRead; } catch (IOException e) { if (currentDataSpecLengthUnset && isCausedByPositionOutOfRange(e)) { - setBytesRemaining(0); + setBytesRemainingAndMaybeStoreLength(0); return C.RESULT_END_OF_INPUT; } handleBeforeThrow(e); @@ -278,12 +283,13 @@ public final class CacheDataSource implements DataSource { @Override public Uri getUri() { - return currentDataSource == upstreamDataSource ? currentDataSource.getUri() : uri; + return actualUri; } @Override public void close() throws IOException { uri = null; + actualUri = null; notifyBytesRead(); try { closeCurrentSource(); @@ -371,7 +377,7 @@ public final class CacheDataSource implements DataSource { ? readPosition + MIN_READ_BEFORE_CHECKING_CACHE : Long.MAX_VALUE; if (checkCache) { - Assertions.checkState(currentDataSource == upstreamDataSource); + Assertions.checkState(isBypassingCache()); if (nextDataSource == upstreamDataSource) { // Continue reading from upstream. return; @@ -395,8 +401,45 @@ public final class CacheDataSource implements DataSource { currentDataSpecLengthUnset = nextDataSpec.length == C.LENGTH_UNSET; long resolvedLength = nextDataSource.open(nextDataSpec); if (currentDataSpecLengthUnset && resolvedLength != C.LENGTH_UNSET) { - setBytesRemaining(resolvedLength); + setBytesRemainingAndMaybeStoreLength(resolvedLength); } + // TODO find a way to store length and redirected uri in one metadata mutation. + maybeUpdateActualUriFieldAndRedirectedUriMetadata(); + } + + private void maybeUpdateActualUriFieldAndRedirectedUriMetadata() { + if (!isReadingFromUpstream()) { + return; + } + actualUri = currentDataSource.getUri(); + maybeUpdateRedirectedUriMetadata(); + } + + private void maybeUpdateRedirectedUriMetadata() { + if (!isWritingToCache()) { + return; + } + ContentMetadataMutations mutations = new ContentMetadataMutations(); + boolean isRedirected = !uri.equals(actualUri); + if (isRedirected) { + mutations.set(ContentMetadata.METADATA_NAME_REDIRECTED_URI, actualUri.toString()); + } else { + mutations.remove(ContentMetadata.METADATA_NAME_REDIRECTED_URI); + } + try { + cache.applyContentMetadataMutations(key, mutations); + } catch (CacheException e) { + String message = + "Couldn't update redirected URI. " + + "This might cause relative URIs get resolved incorrectly."; + Log.w(TAG, message, e); + } + } + + private static Uri loadRedirectedUriOrReturnGivenUri(Cache cache, String key, Uri uri) { + ContentMetadata metadata = cache.getContentMetadata(key); + String redirection = metadata.get(ContentMetadata.METADATA_NAME_REDIRECTED_URI, (String) null); + return redirection == null ? uri : Uri.parse(redirection); } private static boolean isCausedByPositionOutOfRange(IOException e) { @@ -413,13 +456,25 @@ public final class CacheDataSource implements DataSource { return false; } - private void setBytesRemaining(long bytesRemaining) throws IOException { + private void setBytesRemainingAndMaybeStoreLength(long bytesRemaining) throws IOException { this.bytesRemaining = bytesRemaining; if (isWritingToCache()) { cache.setContentLength(key, readPosition + bytesRemaining); } } + private boolean isReadingFromUpstream() { + return !isReadingFromCache(); + } + + private boolean isBypassingCache() { + return currentDataSource == upstreamDataSource; + } + + private boolean isReadingFromCache() { + return currentDataSource == cacheReadDataSource; + } + private boolean isWritingToCache() { return currentDataSource == cacheWriteDataSource; } @@ -441,7 +496,7 @@ public final class CacheDataSource implements DataSource { } private void handleBeforeThrow(IOException exception) { - if (currentDataSource == cacheReadDataSource || exception instanceof CacheException) { + if (isReadingFromCache() || exception instanceof CacheException) { seenCacheError = true; } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContent.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContent.java index 1a9a84c2af..bc17609739 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContent.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CachedContent.java @@ -29,8 +29,7 @@ import java.util.TreeSet; /*package*/ final class CachedContent { private static final int VERSION_METADATA_INTRODUCED = 2; - private static final String EXOPLAYER_METADATA_NAME_PREFIX = "exo_"; - private static final String METADATA_NAME_LENGTH = EXOPLAYER_METADATA_NAME_PREFIX + "len"; + private static final int VERSION_MAX = Integer.MAX_VALUE; /** The cache file id that uniquely identifies the original stream. */ public final int id; @@ -94,21 +93,28 @@ import java.util.TreeSet; return metadata; } - /** Applies {@code mutations} to the metadata. */ - public void applyMetadataMutations(ContentMetadataMutations mutations) { - this.metadata = new DefaultContentMetadata(metadata, mutations); + /** + * Applies {@code mutations} to the metadata. + * + * @return Whether {@code mutations} changed any metadata. + */ + public boolean applyMetadataMutations(ContentMetadataMutations mutations) { + DefaultContentMetadata oldMetadata = metadata; + metadata = metadata.copyWithMutationsApplied(mutations); + return metadata.equals(oldMetadata); } /** * Returns the length of the original stream, or {@link C#LENGTH_UNSET} if the length is unknown. */ public long getLength() { - return metadata.get(METADATA_NAME_LENGTH, C.LENGTH_UNSET); + return metadata.get(ContentMetadata.METADATA_NAME_LENGTH, C.LENGTH_UNSET); } /** Sets the length of the content. */ public void setLength(long length) { - applyMetadataMutations(new ContentMetadataMutations().set(METADATA_NAME_LENGTH, length)); + applyMetadataMutations( + new ContentMetadataMutations().set(ContentMetadata.METADATA_NAME_LENGTH, length)); } /** Returns whether the content is locked. */ @@ -234,4 +240,25 @@ import java.util.TreeSet; return result; } + @Override + public int hashCode() { + int result = headerHashCode(VERSION_MAX); + result = 31 * result + cachedSpans.hashCode(); + return result; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + CachedContent that = (CachedContent) o; + return id == that.id + && key.equals(that.key) + && cachedSpans.equals(that.cachedSpans) + && metadata.equals(that.metadata); + } } 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 32de9fa9c9..c1e8f4b223 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 @@ -224,7 +224,9 @@ import javax.crypto.spec.SecretKeySpec; */ public void applyContentMetadataMutations(String key, ContentMetadataMutations mutations) { CachedContent cachedContent = getOrAdd(key); - cachedContent.applyMetadataMutations(mutations); + if (cachedContent.applyMetadataMutations(mutations)) { + changed = true; + } } /** Returns a {@link ContentMetadata} for the given key. */ diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/ContentMetadata.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/ContentMetadata.java index 190b9f6f38..fba073451f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/ContentMetadata.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/ContentMetadata.java @@ -15,9 +15,21 @@ */ package com.google.android.exoplayer2.upstream.cache; -/** Interface for an immutable snapshot of keyed metadata. */ +/** + * Interface for an immutable snapshot of keyed metadata. + * + *

Internal metadata names are prefixed with {@value #INTERNAL_METADATA_NAME_PREFIX}. Custom + * metadata names should avoid this prefix to prevent clashes. + */ public interface ContentMetadata { + /** Prefix of internal metadata names. */ + String INTERNAL_METADATA_NAME_PREFIX = "exo_"; + /** Name of internal metadata to hold redirected URI. */ + String METADATA_NAME_REDIRECTED_URI = INTERNAL_METADATA_NAME_PREFIX + "redir"; + /** Name of internal metadata to hold content length. */ + String METADATA_NAME_LENGTH = INTERNAL_METADATA_NAME_PREFIX + "len"; + /** * Returns a metadata value. * diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadata.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadata.java index caab925769..b855befe00 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadata.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadata.java @@ -63,18 +63,22 @@ public final class DefaultContentMetadata implements ContentMetadata { private final Map metadata; - /** - * Constructs a {@link DefaultContentMetadata} by copying metadata values from {@code other} and - * applying {@code mutations}. - */ - public DefaultContentMetadata(DefaultContentMetadata other, ContentMetadataMutations mutations) { - this(applyMutations(other.metadata, mutations)); - } - private DefaultContentMetadata(Map metadata) { this.metadata = Collections.unmodifiableMap(metadata); } + /** + * Returns a copy {@link DefaultContentMetadata} with {@code mutations} applied. If {@code + * mutations} don't change anything, returns this instance. + */ + public DefaultContentMetadata copyWithMutationsApplied(ContentMetadataMutations mutations) { + Map mutatedMetadata = applyMutations(metadata, mutations); + if (isMetadataEqual(mutatedMetadata)) { + return this; + } + return new DefaultContentMetadata(mutatedMetadata); + } + /** * Serializes itself to a {@link DataOutputStream}. * @@ -134,8 +138,10 @@ public final class DefaultContentMetadata implements ContentMetadata { if (o == null || getClass() != o.getClass()) { return false; } - DefaultContentMetadata that = (DefaultContentMetadata) o; - Map otherMetadata = that.metadata; + return isMetadataEqual(((DefaultContentMetadata) o).metadata); + } + + private boolean isMetadataEqual(Map otherMetadata) { if (metadata.size() != otherMetadata.size()) { return false; } 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 147689e329..c53f04eb10 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 @@ -309,6 +309,18 @@ public final class SimpleCache implements Cache { return index.getContentLength(key); } + @Override + public void applyContentMetadataMutations(String key, ContentMetadataMutations mutations) + throws CacheException { + index.applyContentMetadataMutations(key, mutations); + index.store(); + } + + @Override + public ContentMetadata getContentMetadata(String key) { + return index.getContentMetadata(key); + } + /** * Returns the cache {@link SimpleCacheSpan} corresponding to the provided lookup {@link * SimpleCacheSpan}. @@ -458,15 +470,4 @@ public final class SimpleCache implements Cache { private static synchronized void releaseFolder(File cacheDir) { lockedCacheDirs.remove(cacheDir.getAbsoluteFile()); } - @Override - public void applyContentMetadataMutations(String key, ContentMetadataMutations mutations) - throws CacheException { - index.applyContentMetadataMutations(key, mutations); - index.store(); - } - - @Override - public ContentMetadata getContentMetadata(String key) { - return index.getContentMetadata(key); - } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadataTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadataTest.java index 02826e8908..e1dc68eac6 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadataTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/DefaultContentMetadataTest.java @@ -62,14 +62,14 @@ public class DefaultContentMetadataTest { @Test public void testEmptyMutationDoesNotFail() throws Exception { ContentMetadataMutations mutations = new ContentMetadataMutations(); - new DefaultContentMetadata(DefaultContentMetadata.EMPTY, mutations); + DefaultContentMetadata.EMPTY.copyWithMutationsApplied(mutations); } @Test public void testAddNewMetadata() throws Exception { ContentMetadataMutations mutations = new ContentMetadataMutations(); mutations.set("metadata name", "value"); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", "default value")).isEqualTo("value"); } @@ -77,7 +77,7 @@ public class DefaultContentMetadataTest { public void testAddNewIntMetadata() throws Exception { ContentMetadataMutations mutations = new ContentMetadataMutations(); mutations.set("metadata name", 5); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", 0)).isEqualTo(5); } @@ -86,7 +86,7 @@ public class DefaultContentMetadataTest { ContentMetadataMutations mutations = new ContentMetadataMutations(); byte[] value = {1, 2, 3}; mutations.set("metadata name", value); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", new byte[] {})).isEqualTo(value); } @@ -102,7 +102,7 @@ public class DefaultContentMetadataTest { contentMetadata = createContentMetadata("metadata name", "value"); ContentMetadataMutations mutations = new ContentMetadataMutations(); mutations.set("metadata name", "edited value"); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", "default value")).isEqualTo("edited value"); } @@ -111,7 +111,7 @@ public class DefaultContentMetadataTest { contentMetadata = createContentMetadata("metadata name", "value"); ContentMetadataMutations mutations = new ContentMetadataMutations(); mutations.remove("metadata name"); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", "default value")).isEqualTo("default value"); } @@ -120,7 +120,7 @@ public class DefaultContentMetadataTest { ContentMetadataMutations mutations = new ContentMetadataMutations(); mutations.set("metadata name", "value"); mutations.remove("metadata name"); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", "default value")).isEqualTo("default value"); } @@ -129,7 +129,7 @@ public class DefaultContentMetadataTest { ContentMetadataMutations mutations = new ContentMetadataMutations(); mutations.remove("metadata name"); mutations.set("metadata name", "value"); - contentMetadata = new DefaultContentMetadata(contentMetadata, mutations); + contentMetadata = contentMetadata.copyWithMutationsApplied(mutations); assertThat(contentMetadata.get("metadata name", "default value")).isEqualTo("value"); } @@ -194,6 +194,6 @@ public class DefaultContentMetadataTest { throw new IllegalArgumentException(); } } - return new DefaultContentMetadata(DefaultContentMetadata.EMPTY, mutations); + return DefaultContentMetadata.EMPTY.copyWithMutationsApplied(mutations); } }