From 14c46bc4062ebc2cf45f96138dc8a5e36bf41da5 Mon Sep 17 00:00:00 2001 From: olly Date: Thu, 23 May 2019 14:59:17 +0100 Subject: [PATCH] Remove contentTypePredicate from DataSource constructors The only known use case for contentTypePredicate is to catch the case when a paywall web page is returned via a DataSource, rather than the data that was being requested. These days streaming providers should be using HTTPS, where this problem does not exist. Devices have also gotten a lot better at showing their own notifications when paywalls are detected, which largely mitigates the need for the app to show a more optimal error message or redirect the user to a browser. It therefore makes sense to deprioritize this feature. In particular by removing the arg from constructors, where nearly all applications are probably passing null. PiperOrigin-RevId: 249634594 --- .../ext/cronet/CronetDataSource.java | 117 +++++++++++++++--- .../ext/cronet/CronetDataSourceFactory.java | 83 +++++-------- .../ext/cronet/CronetDataSourceTest.java | 38 +++--- .../ext/okhttp/OkHttpDataSource.java | 58 +++++++-- .../ext/okhttp/OkHttpDataSourceFactory.java | 1 - .../upstream/DefaultHttpDataSource.java | 75 ++++++++++- .../DefaultHttpDataSourceFactory.java | 1 - 7 files changed, 270 insertions(+), 103 deletions(-) diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index 0ef20e79bd..dd10e5bb66 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -116,7 +116,6 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { private final CronetEngine cronetEngine; private final Executor executor; - @Nullable private final Predicate contentTypePredicate; private final int connectTimeoutMs; private final int readTimeoutMs; private final boolean resetTimeoutOnRedirects; @@ -126,6 +125,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { private final ConditionVariable operation; private final Clock clock; + @Nullable private Predicate contentTypePredicate; + // Accessed by the calling thread only. private boolean opened; private long bytesToSkip; @@ -158,7 +159,78 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { * handling is a fast operation when using a direct executor. */ public CronetDataSource(CronetEngine cronetEngine, Executor executor) { - this(cronetEngine, executor, /* contentTypePredicate= */ null); + this( + cronetEngine, + executor, + DEFAULT_CONNECT_TIMEOUT_MILLIS, + DEFAULT_READ_TIMEOUT_MILLIS, + /* resetTimeoutOnRedirects= */ false, + /* defaultRequestProperties= */ null); + } + + /** + * @param cronetEngine A CronetEngine. + * @param executor The {@link java.util.concurrent.Executor} that will handle responses. This may + * be a direct executor (i.e. executes tasks on the calling thread) in order to avoid a thread + * hop from Cronet's internal network thread to the response handling thread. However, to + * avoid slowing down overall network performance, care must be taken to make sure response + * handling is a fast operation when using a direct executor. + * @param connectTimeoutMs The connection timeout, in milliseconds. + * @param readTimeoutMs The read timeout, in milliseconds. + * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. + * @param defaultRequestProperties Optional default {@link RequestProperties} to be sent to the + * server as HTTP headers on every request. + */ + public CronetDataSource( + CronetEngine cronetEngine, + Executor executor, + int connectTimeoutMs, + int readTimeoutMs, + boolean resetTimeoutOnRedirects, + @Nullable RequestProperties defaultRequestProperties) { + this( + cronetEngine, + executor, + connectTimeoutMs, + readTimeoutMs, + resetTimeoutOnRedirects, + Clock.DEFAULT, + defaultRequestProperties, + /* handleSetCookieRequests= */ false); + } + + /** + * @param cronetEngine A CronetEngine. + * @param executor The {@link java.util.concurrent.Executor} that will handle responses. This may + * be a direct executor (i.e. executes tasks on the calling thread) in order to avoid a thread + * hop from Cronet's internal network thread to the response handling thread. However, to + * avoid slowing down overall network performance, care must be taken to make sure response + * handling is a fast operation when using a direct executor. + * @param connectTimeoutMs The connection timeout, in milliseconds. + * @param readTimeoutMs The read timeout, in milliseconds. + * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. + * @param defaultRequestProperties Optional default {@link RequestProperties} to be sent to the + * server as HTTP headers on every request. + * @param handleSetCookieRequests Whether "Set-Cookie" requests on redirect should be forwarded to + * the redirect url in the "Cookie" header. + */ + public CronetDataSource( + CronetEngine cronetEngine, + Executor executor, + int connectTimeoutMs, + int readTimeoutMs, + boolean resetTimeoutOnRedirects, + @Nullable RequestProperties defaultRequestProperties, + boolean handleSetCookieRequests) { + this( + cronetEngine, + executor, + connectTimeoutMs, + readTimeoutMs, + resetTimeoutOnRedirects, + Clock.DEFAULT, + defaultRequestProperties, + handleSetCookieRequests); } /** @@ -171,7 +243,10 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the * predicate then an {@link InvalidContentTypeException} is thrown from {@link * #open(DataSpec)}. + * @deprecated Use {@link #CronetDataSource(CronetEngine, Executor)} and {@link + * #setContentTypePredicate(Predicate)}. */ + @Deprecated public CronetDataSource( CronetEngine cronetEngine, Executor executor, @@ -182,9 +257,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { contentTypePredicate, DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, - false, - null, - false); + /* resetTimeoutOnRedirects= */ false, + /* defaultRequestProperties= */ null); } /** @@ -200,9 +274,12 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { * @param connectTimeoutMs The connection timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds. * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. - * @param defaultRequestProperties The optional default {@link RequestProperties} to be sent to - * the server as HTTP headers on every request. + * @param defaultRequestProperties Optional default {@link RequestProperties} to be sent to the + * server as HTTP headers on every request. + * @deprecated Use {@link #CronetDataSource(CronetEngine, Executor, int, int, boolean, + * RequestProperties)} and {@link #setContentTypePredicate(Predicate)}. */ + @Deprecated public CronetDataSource( CronetEngine cronetEngine, Executor executor, @@ -218,9 +295,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { connectTimeoutMs, readTimeoutMs, resetTimeoutOnRedirects, - Clock.DEFAULT, defaultRequestProperties, - false); + /* handleSetCookieRequests= */ false); } /** @@ -236,11 +312,14 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { * @param connectTimeoutMs The connection timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds. * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. - * @param defaultRequestProperties The optional default {@link RequestProperties} to be sent to - * the server as HTTP headers on every request. + * @param defaultRequestProperties Optional default {@link RequestProperties} to be sent to the + * server as HTTP headers on every request. * @param handleSetCookieRequests Whether "Set-Cookie" requests on redirect should be forwarded to * the redirect url in the "Cookie" header. + * @deprecated Use {@link #CronetDataSource(CronetEngine, Executor, int, int, boolean, + * RequestProperties, boolean)} and {@link #setContentTypePredicate(Predicate)}. */ + @Deprecated public CronetDataSource( CronetEngine cronetEngine, Executor executor, @@ -253,19 +332,18 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { this( cronetEngine, executor, - contentTypePredicate, connectTimeoutMs, readTimeoutMs, resetTimeoutOnRedirects, Clock.DEFAULT, defaultRequestProperties, handleSetCookieRequests); + this.contentTypePredicate = contentTypePredicate; } /* package */ CronetDataSource( CronetEngine cronetEngine, Executor executor, - @Nullable Predicate contentTypePredicate, int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, @@ -276,7 +354,6 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { this.urlRequestCallback = new UrlRequestCallback(); this.cronetEngine = Assertions.checkNotNull(cronetEngine); this.executor = Assertions.checkNotNull(executor); - this.contentTypePredicate = contentTypePredicate; this.connectTimeoutMs = connectTimeoutMs; this.readTimeoutMs = readTimeoutMs; this.resetTimeoutOnRedirects = resetTimeoutOnRedirects; @@ -287,6 +364,17 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { operation = new ConditionVariable(); } + /** + * Sets a content type {@link Predicate}. If a content type is rejected by the predicate then a + * {@link HttpDataSource.InvalidContentTypeException} is thrown from {@link #open(DataSpec)}. + * + * @param contentTypePredicate The content type {@link Predicate}, or {@code null} to clear a + * predicate that was previously set. + */ + public void setContentTypePredicate(@Nullable Predicate contentTypePredicate) { + this.contentTypePredicate = contentTypePredicate; + } + // HttpDataSource implementation. @Override @@ -363,6 +451,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { } // Check for a valid content type. + Predicate contentTypePredicate = this.contentTypePredicate; if (contentTypePredicate != null) { List contentTypeHeaders = responseInfo.getAllHeaders().get(CONTENT_TYPE); String contentType = isEmpty(contentTypeHeaders) ? null : contentTypeHeaders.get(0); diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java index 93edb4e893..4086011b4f 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceFactory.java @@ -20,9 +20,7 @@ import com.google.android.exoplayer2.upstream.DefaultHttpDataSourceFactory; import com.google.android.exoplayer2.upstream.HttpDataSource; import com.google.android.exoplayer2.upstream.HttpDataSource.BaseFactory; import com.google.android.exoplayer2.upstream.HttpDataSource.Factory; -import com.google.android.exoplayer2.upstream.HttpDataSource.InvalidContentTypeException; import com.google.android.exoplayer2.upstream.TransferListener; -import com.google.android.exoplayer2.util.Predicate; import java.util.concurrent.Executor; import org.chromium.net.CronetEngine; @@ -45,8 +43,7 @@ public final class CronetDataSourceFactory extends BaseFactory { private final CronetEngineWrapper cronetEngineWrapper; private final Executor executor; - private final Predicate contentTypePredicate; - private final @Nullable TransferListener transferListener; + @Nullable private final TransferListener transferListener; private final int connectTimeoutMs; private final int readTimeoutMs; private final boolean resetTimeoutOnRedirects; @@ -64,21 +61,16 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param fallbackFactory A {@link HttpDataSource.Factory} which is used as a fallback in case no * suitable CronetEngine can be build. */ public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, HttpDataSource.Factory fallbackFactory) { this( cronetEngineWrapper, executor, - contentTypePredicate, /* transferListener= */ null, DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, @@ -98,20 +90,15 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param userAgent A user agent used to create a fallback HttpDataSource if needed. */ public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, String userAgent) { this( cronetEngineWrapper, executor, - contentTypePredicate, /* transferListener= */ null, DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, @@ -132,9 +119,6 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param connectTimeoutMs The connection timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds. * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. @@ -143,7 +127,6 @@ public final class CronetDataSourceFactory extends BaseFactory { public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, @@ -151,7 +134,6 @@ public final class CronetDataSourceFactory extends BaseFactory { this( cronetEngineWrapper, executor, - contentTypePredicate, /* transferListener= */ null, DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, @@ -172,9 +154,6 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param connectTimeoutMs The connection timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds. * @param resetTimeoutOnRedirects Whether the connect timeout is reset when a redirect occurs. @@ -184,7 +163,6 @@ public final class CronetDataSourceFactory extends BaseFactory { public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, @@ -192,7 +170,6 @@ public final class CronetDataSourceFactory extends BaseFactory { this( cronetEngineWrapper, executor, - contentTypePredicate, /* transferListener= */ null, connectTimeoutMs, readTimeoutMs, @@ -212,9 +189,6 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param transferListener An optional listener. * @param fallbackFactory A {@link HttpDataSource.Factory} which is used as a fallback in case no * suitable CronetEngine can be build. @@ -222,11 +196,16 @@ public final class CronetDataSourceFactory extends BaseFactory { public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, @Nullable TransferListener transferListener, HttpDataSource.Factory fallbackFactory) { - this(cronetEngineWrapper, executor, contentTypePredicate, transferListener, - DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, false, fallbackFactory); + this( + cronetEngineWrapper, + executor, + transferListener, + DEFAULT_CONNECT_TIMEOUT_MILLIS, + DEFAULT_READ_TIMEOUT_MILLIS, + false, + fallbackFactory); } /** @@ -241,22 +220,27 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param transferListener An optional listener. * @param userAgent A user agent used to create a fallback HttpDataSource if needed. */ public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, @Nullable TransferListener transferListener, String userAgent) { - this(cronetEngineWrapper, executor, contentTypePredicate, transferListener, - DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, false, - new DefaultHttpDataSourceFactory(userAgent, transferListener, - DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, false)); + this( + cronetEngineWrapper, + executor, + transferListener, + DEFAULT_CONNECT_TIMEOUT_MILLIS, + DEFAULT_READ_TIMEOUT_MILLIS, + false, + new DefaultHttpDataSourceFactory( + userAgent, + transferListener, + DEFAULT_CONNECT_TIMEOUT_MILLIS, + DEFAULT_READ_TIMEOUT_MILLIS, + false)); } /** @@ -267,9 +251,6 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param transferListener An optional listener. * @param connectTimeoutMs The connection timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds. @@ -279,16 +260,20 @@ public final class CronetDataSourceFactory extends BaseFactory { public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, @Nullable TransferListener transferListener, int connectTimeoutMs, int readTimeoutMs, boolean resetTimeoutOnRedirects, String userAgent) { - this(cronetEngineWrapper, executor, contentTypePredicate, transferListener, - DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS, resetTimeoutOnRedirects, - new DefaultHttpDataSourceFactory(userAgent, transferListener, connectTimeoutMs, - readTimeoutMs, resetTimeoutOnRedirects)); + this( + cronetEngineWrapper, + executor, + transferListener, + DEFAULT_CONNECT_TIMEOUT_MILLIS, + DEFAULT_READ_TIMEOUT_MILLIS, + resetTimeoutOnRedirects, + new DefaultHttpDataSourceFactory( + userAgent, transferListener, connectTimeoutMs, readTimeoutMs, resetTimeoutOnRedirects)); } /** @@ -299,9 +284,6 @@ public final class CronetDataSourceFactory extends BaseFactory { * * @param cronetEngineWrapper A {@link CronetEngineWrapper}. * @param executor The {@link java.util.concurrent.Executor} that will perform the requests. - * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the - * predicate then an {@link InvalidContentTypeException} is thrown from {@link - * CronetDataSource#open}. * @param transferListener An optional listener. * @param connectTimeoutMs The connection timeout, in milliseconds. * @param readTimeoutMs The read timeout, in milliseconds. @@ -312,7 +294,6 @@ public final class CronetDataSourceFactory extends BaseFactory { public CronetDataSourceFactory( CronetEngineWrapper cronetEngineWrapper, Executor executor, - Predicate contentTypePredicate, @Nullable TransferListener transferListener, int connectTimeoutMs, int readTimeoutMs, @@ -320,7 +301,6 @@ public final class CronetDataSourceFactory extends BaseFactory { HttpDataSource.Factory fallbackFactory) { this.cronetEngineWrapper = cronetEngineWrapper; this.executor = executor; - this.contentTypePredicate = contentTypePredicate; this.transferListener = transferListener; this.connectTimeoutMs = connectTimeoutMs; this.readTimeoutMs = readTimeoutMs; @@ -339,7 +319,6 @@ public final class CronetDataSourceFactory extends BaseFactory { new CronetDataSource( cronetEngine, executor, - contentTypePredicate, connectTimeoutMs, readTimeoutMs, resetTimeoutOnRedirects, diff --git a/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java b/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java index 7c4c03dd87..df36076899 100644 --- a/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java +++ b/extensions/cronet/src/test/java/com/google/android/exoplayer2/ext/cronet/CronetDataSourceTest.java @@ -38,7 +38,6 @@ import com.google.android.exoplayer2.upstream.HttpDataSource; import com.google.android.exoplayer2.upstream.HttpDataSource.HttpDataSourceException; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.util.Clock; -import com.google.android.exoplayer2.util.Predicate; import com.google.android.exoplayer2.util.Util; import java.io.IOException; import java.net.SocketTimeoutException; @@ -85,7 +84,6 @@ public final class CronetDataSourceTest { @Mock private UrlRequest.Builder mockUrlRequestBuilder; @Mock private UrlRequest mockUrlRequest; - @Mock private Predicate mockContentTypePredicate; @Mock private TransferListener mockTransferListener; @Mock private Executor mockExecutor; @Mock private NetworkException mockNetworkException; @@ -95,21 +93,19 @@ public final class CronetDataSourceTest { private boolean redirectCalled; @Before - public void setUp() throws Exception { + public void setUp() { MockitoAnnotations.initMocks(this); dataSourceUnderTest = new CronetDataSource( mockCronetEngine, mockExecutor, - mockContentTypePredicate, TEST_CONNECT_TIMEOUT_MS, TEST_READ_TIMEOUT_MS, - true, // resetTimeoutOnRedirects + /* resetTimeoutOnRedirects= */ true, Clock.DEFAULT, - null, - false); + /* defaultRequestProperties= */ null, + /* handleSetCookieRequests= */ false); dataSourceUnderTest.addTransferListener(mockTransferListener); - when(mockContentTypePredicate.evaluate(anyString())).thenReturn(true); when(mockCronetEngine.newUrlRequestBuilder( anyString(), any(UrlRequest.Callback.class), any(Executor.class))) .thenReturn(mockUrlRequestBuilder); @@ -283,7 +279,13 @@ public final class CronetDataSourceTest { @Test public void testRequestOpenValidatesContentTypePredicate() { mockResponseStartSuccess(); - when(mockContentTypePredicate.evaluate(anyString())).thenReturn(false); + + ArrayList testedContentTypes = new ArrayList<>(); + dataSourceUnderTest.setContentTypePredicate( + (String input) -> { + testedContentTypes.add(input); + return false; + }); try { dataSourceUnderTest.open(testDataSpec); @@ -292,7 +294,8 @@ public final class CronetDataSourceTest { assertThat(e instanceof HttpDataSource.InvalidContentTypeException).isTrue(); // Check for connection not automatically closed. verify(mockUrlRequest, never()).cancel(); - verify(mockContentTypePredicate).evaluate(TEST_CONTENT_TYPE); + assertThat(testedContentTypes).hasSize(1); + assertThat(testedContentTypes.get(0)).isEqualTo(TEST_CONTENT_TYPE); } } @@ -734,7 +737,6 @@ public final class CronetDataSourceTest { new CronetDataSource( mockCronetEngine, mockExecutor, - mockContentTypePredicate, TEST_CONNECT_TIMEOUT_MS, TEST_READ_TIMEOUT_MS, true, // resetTimeoutOnRedirects @@ -765,13 +767,12 @@ public final class CronetDataSourceTest { new CronetDataSource( mockCronetEngine, mockExecutor, - mockContentTypePredicate, TEST_CONNECT_TIMEOUT_MS, TEST_READ_TIMEOUT_MS, - true, // resetTimeoutOnRedirects + /* resetTimeoutOnRedirects= */ true, Clock.DEFAULT, - null, - true); + /* defaultRequestProperties= */ null, + /* handleSetCookieRequests= */ true); dataSourceUnderTest.addTransferListener(mockTransferListener); dataSourceUnderTest.setRequestProperty("Content-Type", TEST_CONTENT_TYPE); @@ -804,13 +805,12 @@ public final class CronetDataSourceTest { new CronetDataSource( mockCronetEngine, mockExecutor, - mockContentTypePredicate, TEST_CONNECT_TIMEOUT_MS, TEST_READ_TIMEOUT_MS, - true, // resetTimeoutOnRedirects + /* resetTimeoutOnRedirects= */ true, Clock.DEFAULT, - null, - true); + /* defaultRequestProperties= */ null, + /* handleSetCookieRequests= */ true); dataSourceUnderTest.addTransferListener(mockTransferListener); mockSingleRedirectSuccess(); mockFollowRedirectSuccess(); diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index 8eb8bba920..eaa305875b 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -57,14 +57,14 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { private final Call.Factory callFactory; private final RequestProperties requestProperties; - private final @Nullable String userAgent; - private final @Nullable Predicate contentTypePredicate; - private final @Nullable CacheControl cacheControl; - private final @Nullable RequestProperties defaultRequestProperties; + @Nullable private final String userAgent; + @Nullable private final CacheControl cacheControl; + @Nullable private final RequestProperties defaultRequestProperties; - private @Nullable DataSpec dataSpec; - private @Nullable Response response; - private @Nullable InputStream responseByteStream; + @Nullable private Predicate contentTypePredicate; + @Nullable private DataSpec dataSpec; + @Nullable private Response response; + @Nullable private InputStream responseByteStream; private boolean opened; private long bytesToSkip; @@ -79,7 +79,28 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { * @param userAgent An optional User-Agent string. */ public OkHttpDataSource(Call.Factory callFactory, @Nullable String userAgent) { - this(callFactory, userAgent, /* contentTypePredicate= */ null); + this(callFactory, userAgent, /* cacheControl= */ null, /* defaultRequestProperties= */ null); + } + + /** + * @param callFactory A {@link Call.Factory} (typically an {@link okhttp3.OkHttpClient}) for use + * by the source. + * @param userAgent An optional User-Agent string. + * @param cacheControl An optional {@link CacheControl} for setting the Cache-Control header. + * @param defaultRequestProperties Optional default {@link RequestProperties} to be sent to the + * server as HTTP headers on every request. + */ + public OkHttpDataSource( + Call.Factory callFactory, + @Nullable String userAgent, + @Nullable CacheControl cacheControl, + @Nullable RequestProperties defaultRequestProperties) { + super(/* isNetwork= */ true); + this.callFactory = Assertions.checkNotNull(callFactory); + this.userAgent = userAgent; + this.cacheControl = cacheControl; + this.defaultRequestProperties = defaultRequestProperties; + this.requestProperties = new RequestProperties(); } /** @@ -89,7 +110,10 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the * predicate then a {@link InvalidContentTypeException} is thrown from {@link * #open(DataSpec)}. + * @deprecated Use {@link #OkHttpDataSource(Call.Factory, String)} and {@link + * #setContentTypePredicate(Predicate)}. */ + @Deprecated public OkHttpDataSource( Call.Factory callFactory, @Nullable String userAgent, @@ -110,9 +134,12 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { * predicate then a {@link InvalidContentTypeException} is thrown from {@link * #open(DataSpec)}. * @param cacheControl An optional {@link CacheControl} for setting the Cache-Control header. - * @param defaultRequestProperties The optional default {@link RequestProperties} to be sent to - * the server as HTTP headers on every request. + * @param defaultRequestProperties Optional default {@link RequestProperties} to be sent to the + * server as HTTP headers on every request. + * @deprecated Use {@link #OkHttpDataSource(Call.Factory, String, CacheControl, + * RequestProperties)} and {@link #setContentTypePredicate(Predicate)}. */ + @Deprecated public OkHttpDataSource( Call.Factory callFactory, @Nullable String userAgent, @@ -128,6 +155,17 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { this.requestProperties = new RequestProperties(); } + /** + * Sets a content type {@link Predicate}. If a content type is rejected by the predicate then a + * {@link HttpDataSource.InvalidContentTypeException} is thrown from {@link #open(DataSpec)}. + * + * @param contentTypePredicate The content type {@link Predicate}, or {@code null} to clear a + * predicate that was previously set. + */ + public void setContentTypePredicate(@Nullable Predicate contentTypePredicate) { + this.contentTypePredicate = contentTypePredicate; + } + @Override public @Nullable Uri getUri() { return response == null ? null : Uri.parse(response.request().url().toString()); diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java index d0ef35cb07..f18e37c5c4 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSourceFactory.java @@ -89,7 +89,6 @@ public final class OkHttpDataSourceFactory extends BaseFactory { new OkHttpDataSource( callFactory, userAgent, - /* contentTypePredicate= */ null, cacheControl, defaultRequestProperties); if (listener != null) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java index 65b65efe2c..5955a5d9d9 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java @@ -74,13 +74,13 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou private final int connectTimeoutMillis; private final int readTimeoutMillis; private final String userAgent; - private final @Nullable Predicate contentTypePredicate; - private final @Nullable RequestProperties defaultRequestProperties; + @Nullable private final RequestProperties defaultRequestProperties; private final RequestProperties requestProperties; - private @Nullable DataSpec dataSpec; - private @Nullable HttpURLConnection connection; - private @Nullable InputStream inputStream; + @Nullable private Predicate contentTypePredicate; + @Nullable private DataSpec dataSpec; + @Nullable private HttpURLConnection connection; + @Nullable private InputStream inputStream; private boolean opened; private long bytesToSkip; @@ -91,7 +91,50 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou /** @param userAgent The User-Agent string that should be used. */ public DefaultHttpDataSource(String userAgent) { - this(userAgent, /* contentTypePredicate= */ null); + this(userAgent, DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS); + } + + /** + * @param userAgent The User-Agent string that should be used. + * @param connectTimeoutMillis The connection timeout, in milliseconds. A timeout of zero is + * interpreted as an infinite timeout. + * @param readTimeoutMillis The read timeout, in milliseconds. A timeout of zero is interpreted as + * an infinite timeout. + */ + public DefaultHttpDataSource(String userAgent, int connectTimeoutMillis, int readTimeoutMillis) { + this( + userAgent, + connectTimeoutMillis, + readTimeoutMillis, + /* allowCrossProtocolRedirects= */ false, + /* defaultRequestProperties= */ null); + } + + /** + * @param userAgent The User-Agent string that should be used. + * @param connectTimeoutMillis The connection timeout, in milliseconds. A timeout of zero is + * interpreted as an infinite timeout. Pass {@link #DEFAULT_CONNECT_TIMEOUT_MILLIS} to use the + * default value. + * @param readTimeoutMillis The read timeout, in milliseconds. A timeout of zero is interpreted as + * an infinite timeout. Pass {@link #DEFAULT_READ_TIMEOUT_MILLIS} to use the default value. + * @param allowCrossProtocolRedirects Whether cross-protocol redirects (i.e. redirects from HTTP + * to HTTPS and vice versa) are enabled. + * @param defaultRequestProperties The default request properties to be sent to the server as HTTP + * headers or {@code null} if not required. + */ + public DefaultHttpDataSource( + String userAgent, + int connectTimeoutMillis, + int readTimeoutMillis, + boolean allowCrossProtocolRedirects, + @Nullable RequestProperties defaultRequestProperties) { + super(/* isNetwork= */ true); + this.userAgent = Assertions.checkNotEmpty(userAgent); + this.requestProperties = new RequestProperties(); + this.connectTimeoutMillis = connectTimeoutMillis; + this.readTimeoutMillis = readTimeoutMillis; + this.allowCrossProtocolRedirects = allowCrossProtocolRedirects; + this.defaultRequestProperties = defaultRequestProperties; } /** @@ -99,7 +142,10 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou * @param contentTypePredicate An optional {@link Predicate}. If a content type is rejected by the * predicate then a {@link HttpDataSource.InvalidContentTypeException} is thrown from {@link * #open(DataSpec)}. + * @deprecated Use {@link #DefaultHttpDataSource(String)} and {@link + * #setContentTypePredicate(Predicate)}. */ + @Deprecated public DefaultHttpDataSource(String userAgent, @Nullable Predicate contentTypePredicate) { this( userAgent, @@ -117,7 +163,10 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou * interpreted as an infinite timeout. * @param readTimeoutMillis The read timeout, in milliseconds. A timeout of zero is interpreted as * an infinite timeout. + * @deprecated Use {@link #DefaultHttpDataSource(String, int, int)} and {@link + * #setContentTypePredicate(Predicate)}. */ + @Deprecated public DefaultHttpDataSource( String userAgent, @Nullable Predicate contentTypePredicate, @@ -146,7 +195,10 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou * to HTTPS and vice versa) are enabled. * @param defaultRequestProperties The default request properties to be sent to the server as HTTP * headers or {@code null} if not required. + * @deprecated Use {@link #DefaultHttpDataSource(String, int, int, boolean, RequestProperties)} + * and {@link #setContentTypePredicate(Predicate)}. */ + @Deprecated public DefaultHttpDataSource( String userAgent, @Nullable Predicate contentTypePredicate, @@ -164,6 +216,17 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou this.defaultRequestProperties = defaultRequestProperties; } + /** + * Sets a content type {@link Predicate}. If a content type is rejected by the predicate then a + * {@link HttpDataSource.InvalidContentTypeException} is thrown from {@link #open(DataSpec)}. + * + * @param contentTypePredicate The content type {@link Predicate}, or {@code null} to clear a + * predicate that was previously set. + */ + public void setContentTypePredicate(@Nullable Predicate contentTypePredicate) { + this.contentTypePredicate = contentTypePredicate; + } + @Override public @Nullable Uri getUri() { return connection == null ? null : Uri.parse(connection.getURL().toString()); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceFactory.java index 371343857f..e0b1efad54 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceFactory.java @@ -107,7 +107,6 @@ public final class DefaultHttpDataSourceFactory extends BaseFactory { DefaultHttpDataSource dataSource = new DefaultHttpDataSource( userAgent, - /* contentTypePredicate= */ null, connectTimeoutMillis, readTimeoutMillis, allowCrossProtocolRedirects,