From 04824dfd250234275b1cf4979e29fa376df38185 Mon Sep 17 00:00:00 2001 From: bachinger Date: Mon, 21 Dec 2020 11:25:11 +0000 Subject: [PATCH] Make DefaultHttpDataSourceFactory an inner class of the built class #exofixit PiperOrigin-RevId: 348441436 --- RELEASENOTES.md | 2 + .../upstream/DefaultDataSource.java | 12 +- .../upstream/DefaultDataSourceFactory.java | 2 +- .../upstream/DefaultHttpDataSource.java | 277 ++++++++++-------- .../DefaultHttpDataSourceFactory.java | 6 +- .../upstream/DefaultHttpDataSourceTest.java | 55 ++-- .../exoplayer2/testutil/FakeMediaChunk.java | 3 +- 7 files changed, 210 insertions(+), 147 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 67abce015b..5816b1bd29 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -27,6 +27,8 @@ `HttpDataSource.Factory.setDefaultRequestProperties` instead. * Fix playback issues after seeking during an ad ([#8349](https://github.com/google/ExoPlayer/issues/8349)) + * Add `DefaultHttpDataSource.Factory` and deprecate + `DefaultHttpDataSourceFactory`. * Track selection: * Add option to specify multiple preferred audio or text languages. * Forward `Timeline` and `MediaPeriodId` to `TrackSelection.Factory`. diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java index 12fea3898c..534378a635 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSource.java @@ -132,12 +132,12 @@ public final class DefaultDataSource implements DataSource { boolean allowCrossProtocolRedirects) { this( context, - new DefaultHttpDataSource( - userAgent, - connectTimeoutMillis, - readTimeoutMillis, - allowCrossProtocolRedirects, - /* defaultRequestProperties= */ null)); + new DefaultHttpDataSource.Factory() + .setUserAgent(userAgent) + .setConnectTimeoutMs(connectTimeoutMillis) + .setReadTimeoutMs(readTimeoutMillis) + .setAllowCrossProtocolRedirects(allowCrossProtocolRedirects) + .createDataSource()); } /** diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSourceFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSourceFactory.java index 68ce25c47f..a13008c338 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSourceFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultDataSourceFactory.java @@ -59,7 +59,7 @@ public final class DefaultDataSourceFactory implements Factory { */ public DefaultDataSourceFactory( Context context, String userAgent, @Nullable TransferListener listener) { - this(context, listener, new DefaultHttpDataSourceFactory(userAgent, listener)); + this(context, listener, new DefaultHttpDataSource.Factory().setUserAgent(userAgent)); } /** 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 94c02c7e83..15bee2c118 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 @@ -15,6 +15,7 @@ */ package com.google.android.exoplayer2.upstream; +import static com.google.android.exoplayer2.ExoPlayerLibraryInfo.DEFAULT_USER_AGENT; import static java.lang.Math.max; import static java.lang.Math.min; @@ -52,16 +53,148 @@ import java.util.zip.GZIPInputStream; * An {@link HttpDataSource} that uses Android's {@link HttpURLConnection}. * *

By default this implementation will not follow cross-protocol redirects (i.e. redirects from - * HTTP to HTTPS or vice versa). Cross-protocol redirects can be enabled by using the {@link - * #DefaultHttpDataSource(String, int, int, boolean, RequestProperties)} constructor and passing - * {@code true} for the {@code allowCrossProtocolRedirects} argument. + * HTTP to HTTPS or vice versa). Cross-protocol redirects can be enabled by passing {@code true} to + * {@link DefaultHttpDataSource.Factory#setAllowCrossProtocolRedirects(boolean)}. * *

Note: HTTP request headers will be set using all parameters passed via (in order of decreasing - * priority) the {@code dataSpec}, {@link #setRequestProperty} and the default parameters used to - * construct the instance. + * priority) the {@code dataSpec}, {@link #setRequestProperty} and the default properties that can + * be passed to {@link HttpDataSource.Factory#setDefaultRequestProperties(Map)}. */ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSource { + /** {@link DataSource.Factory} for {@link DefaultHttpDataSource} instances. */ + public static final class Factory implements HttpDataSource.Factory { + + private final RequestProperties defaultRequestProperties; + + @Nullable private TransferListener transferListener; + @Nullable private Predicate contentTypePredicate; + private String userAgent; + private int connectTimeoutMs; + private int readTimeoutMs; + private boolean allowCrossProtocolRedirects; + + /** Creates an instance. */ + public Factory() { + defaultRequestProperties = new RequestProperties(); + userAgent = DEFAULT_USER_AGENT; + connectTimeoutMs = DEFAULT_CONNECT_TIMEOUT_MILLIS; + readTimeoutMs = DEFAULT_READ_TIMEOUT_MILLIS; + } + + /** @deprecated Use {@link #setDefaultRequestProperties(Map)} instead. */ + @Deprecated + @Override + public final RequestProperties getDefaultRequestProperties() { + return defaultRequestProperties; + } + + @Override + public final Factory setDefaultRequestProperties(Map defaultRequestProperties) { + this.defaultRequestProperties.clearAndSet(defaultRequestProperties); + return this; + } + + /** + * Sets the user agent that will be used. + * + *

The default is {@link ExoPlayerLibraryInfo#DEFAULT_USER_AGENT}. + * + * @param userAgent The user agent that will be used. + * @return This factory. + */ + public Factory setUserAgent(String userAgent) { + this.userAgent = userAgent; + return this; + } + + /** + * Sets the connect timeout, in milliseconds. + * + *

The default is {@link DefaultHttpDataSource#DEFAULT_CONNECT_TIMEOUT_MILLIS}. + * + * @param connectTimeoutMs The connect timeout, in milliseconds, that will be used. + * @return This factory. + */ + public Factory setConnectTimeoutMs(int connectTimeoutMs) { + this.connectTimeoutMs = connectTimeoutMs; + return this; + } + + /** + * Sets the read timeout, in milliseconds. + * + *

The default is {@link DefaultHttpDataSource#DEFAULT_READ_TIMEOUT_MILLIS}. + * + * @param readTimeoutMs The connect timeout, in milliseconds, that will be used. + * @return This factory. + */ + public Factory setReadTimeoutMs(int readTimeoutMs) { + this.readTimeoutMs = readTimeoutMs; + return this; + } + + /** + * Sets whether to allow cross protocol redirects. + * + *

The default is {@code false}. + * + * @param allowCrossProtocolRedirects Whether to allow cross protocol redirects. + * @return This factory. + */ + public Factory setAllowCrossProtocolRedirects(boolean allowCrossProtocolRedirects) { + this.allowCrossProtocolRedirects = allowCrossProtocolRedirects; + return this; + } + + /** + * Sets a content type {@link Predicate}. If a content type is rejected by the predicate then a + * {@link HttpDataSource.InvalidContentTypeException} is thrown from {@link + * DefaultHttpDataSource#open(DataSpec)}. + * + *

The default is {@code null}. + * + * @param contentTypePredicate The content type {@link Predicate}, or {@code null} to clear a + * predicate that was previously set. + * @return This factory. + */ + public Factory setContentTypePredicate(@Nullable Predicate contentTypePredicate) { + this.contentTypePredicate = contentTypePredicate; + return this; + } + + /** + * Sets the {@link TransferListener} that will be used. + * + *

The default is {@code null}. + * + *

See {@link DataSource#addTransferListener(TransferListener)}. + * + * @param transferListener The listener that will be used. + * @return This factory. + */ + public Factory setTransferListener(@Nullable TransferListener transferListener) { + this.transferListener = transferListener; + return this; + } + + @Override + public DefaultHttpDataSource createDataSource() { + DefaultHttpDataSource dataSource = + new DefaultHttpDataSource( + userAgent, + connectTimeoutMs, + readTimeoutMs, + allowCrossProtocolRedirects, + defaultRequestProperties, + contentTypePredicate); + if (transferListener != null) { + dataSource.addTransferListener(transferListener); + } + return dataSource; + } + } + /** The default connection timeout, in milliseconds. */ public static final int DEFAULT_CONNECT_TIMEOUT_MILLIS = 8 * 1000; /** @@ -98,7 +231,9 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou private long bytesSkipped; private long bytesRead; - /** Creates an instance. */ + /** @deprecated Use {@link DefaultHttpDataSource.Factory} instead. */ + @SuppressWarnings("deprecation") + @Deprecated public DefaultHttpDataSource() { this( ExoPlayerLibraryInfo.DEFAULT_USER_AGENT, @@ -106,24 +241,16 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou DEFAULT_READ_TIMEOUT_MILLIS); } - /** - * Creates an instance. - * - * @param userAgent The User-Agent string that should be used. - */ + /** @deprecated Use {@link DefaultHttpDataSource.Factory} instead. */ + @SuppressWarnings("deprecation") + @Deprecated public DefaultHttpDataSource(String userAgent) { this(userAgent, DEFAULT_CONNECT_TIMEOUT_MILLIS, DEFAULT_READ_TIMEOUT_MILLIS); } - /** - * Creates an instance. - * - * @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. - */ + /** @deprecated Use {@link DefaultHttpDataSource.Factory} instead. */ + @SuppressWarnings("deprecation") + @Deprecated public DefaultHttpDataSource(String userAgent, int connectTimeoutMillis, int readTimeoutMillis) { this( userAgent, @@ -133,129 +260,45 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou /* defaultRequestProperties= */ null); } - /** - * Creates an instance. - * - * @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. - */ + /** @deprecated Use {@link DefaultHttpDataSource.Factory} instead. */ + @Deprecated 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; - } - - /** - * Creates an instance. - * - * @param userAgent The User-Agent string that should be used. - * @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)}. - */ - @SuppressWarnings("deprecation") - @Deprecated - public DefaultHttpDataSource(String userAgent, @Nullable Predicate contentTypePredicate) { this( userAgent, - contentTypePredicate, - DEFAULT_CONNECT_TIMEOUT_MILLIS, - DEFAULT_READ_TIMEOUT_MILLIS); - } - - /** - * Creates an instance. - * - * @param userAgent The User-Agent string that should be used. - * @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)}. - * @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. - * @deprecated Use {@link #DefaultHttpDataSource(String, int, int)} and {@link - * #setContentTypePredicate(Predicate)}. - */ - @SuppressWarnings("deprecation") - @Deprecated - public DefaultHttpDataSource( - String userAgent, - @Nullable Predicate contentTypePredicate, - int connectTimeoutMillis, - int readTimeoutMillis) { - this( - userAgent, - contentTypePredicate, connectTimeoutMillis, readTimeoutMillis, - /* allowCrossProtocolRedirects= */ false, - /* defaultRequestProperties= */ null); + allowCrossProtocolRedirects, + defaultRequestProperties, + /* contentTypePredicate= */ null); } - /** - * Creates an instance. - * - * @param userAgent The User-Agent string that should be used. - * @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)}. - * @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. - * @deprecated Use {@link #DefaultHttpDataSource(String, int, int, boolean, RequestProperties)} - * and {@link #setContentTypePredicate(Predicate)}. - */ - @Deprecated - public DefaultHttpDataSource( + private DefaultHttpDataSource( String userAgent, - @Nullable Predicate contentTypePredicate, int connectTimeoutMillis, int readTimeoutMillis, boolean allowCrossProtocolRedirects, - @Nullable RequestProperties defaultRequestProperties) { + @Nullable RequestProperties defaultRequestProperties, + @Nullable Predicate contentTypePredicate) { super(/* isNetwork= */ true); this.userAgent = Assertions.checkNotEmpty(userAgent); - this.contentTypePredicate = contentTypePredicate; - this.requestProperties = new RequestProperties(); this.connectTimeoutMillis = connectTimeoutMillis; this.readTimeoutMillis = readTimeoutMillis; this.allowCrossProtocolRedirects = allowCrossProtocolRedirects; this.defaultRequestProperties = defaultRequestProperties; + this.contentTypePredicate = contentTypePredicate; + 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. + * @deprecated Use {@link DefaultHttpDataSource.Factory#setContentTypePredicate(Predicate)} + * instead. */ + @Deprecated public void setContentTypePredicate(@Nullable Predicate contentTypePredicate) { this.contentTypePredicate = contentTypePredicate; } 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 0a0650a4b1..82fad457fb 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 @@ -19,10 +19,10 @@ import static com.google.android.exoplayer2.ExoPlayerLibraryInfo.DEFAULT_USER_AG import androidx.annotation.Nullable; import com.google.android.exoplayer2.upstream.HttpDataSource.BaseFactory; -import com.google.android.exoplayer2.upstream.HttpDataSource.Factory; import com.google.android.exoplayer2.util.Assertions; -/** A {@link Factory} that produces {@link DefaultHttpDataSource} instances. */ +/** @deprecated Use {@link DefaultHttpDataSource.Factory} instead. */ +@Deprecated public final class DefaultHttpDataSourceFactory extends BaseFactory { private final String userAgent; @@ -110,6 +110,8 @@ public final class DefaultHttpDataSourceFactory extends BaseFactory { this.allowCrossProtocolRedirects = allowCrossProtocolRedirects; } + // Calls deprecated constructor. + @SuppressWarnings("deprecation") @Override protected DefaultHttpDataSource createDataSourceInternal( HttpDataSource.RequestProperties defaultRequestProperties) { diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java index 8d5a7479e5..f84b448977 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSourceTest.java @@ -57,19 +57,18 @@ public class DefaultHttpDataSourceTest { MockWebServer mockWebServer = new MockWebServer(); mockWebServer.enqueue(new MockResponse()); - String propertyFromConstructor = "fromConstructor"; - HttpDataSource.RequestProperties constructorProperties = new HttpDataSource.RequestProperties(); - constructorProperties.set("0", propertyFromConstructor); - constructorProperties.set("1", propertyFromConstructor); - constructorProperties.set("2", propertyFromConstructor); - constructorProperties.set("4", propertyFromConstructor); + String propertyFromFactory = "fromFactory"; + Map defaultRequestProperties = new HashMap<>(); + defaultRequestProperties.put("0", propertyFromFactory); + defaultRequestProperties.put("1", propertyFromFactory); + defaultRequestProperties.put("2", propertyFromFactory); + defaultRequestProperties.put("4", propertyFromFactory); DefaultHttpDataSource dataSource = - new DefaultHttpDataSource( - /* userAgent= */ "testAgent", - /* connectTimeoutMillis= */ 1000, - /* readTimeoutMillis= */ 1000, - /* allowCrossProtocolRedirects= */ false, - constructorProperties); + new DefaultHttpDataSource.Factory() + .setConnectTimeoutMs(1000) + .setReadTimeoutMs(1000) + .setDefaultRequestProperties(defaultRequestProperties) + .createDataSource(); String propertyFromSetter = "fromSetter"; dataSource.setRequestProperty("1", propertyFromSetter); @@ -92,7 +91,7 @@ public class DefaultHttpDataSourceTest { dataSource.open(dataSpec); Headers headers = mockWebServer.takeRequest(10, SECONDS).getHeaders(); - assertThat(headers.get("0")).isEqualTo(propertyFromConstructor); + assertThat(headers.get("0")).isEqualTo(propertyFromFactory); assertThat(headers.get("1")).isEqualTo(propertyFromSetter); assertThat(headers.get("2")).isEqualTo(propertyFromDataSpec); assertThat(headers.get("3")).isEqualTo(propertyFromDataSpec); @@ -102,14 +101,12 @@ public class DefaultHttpDataSourceTest { } @Test - public void open_invalidResponseCode() throws Exception { + public void open_invalidResponseCode() { DefaultHttpDataSource defaultHttpDataSource = - new DefaultHttpDataSource( - /* userAgent= */ "testAgent", - /* connectTimeoutMillis= */ 1000, - /* readTimeoutMillis= */ 1000, - /* allowCrossProtocolRedirects= */ false, - /* defaultRequestProperties= */ null); + new DefaultHttpDataSource.Factory() + .setConnectTimeoutMs(1000) + .setReadTimeoutMs(1000) + .createDataSource(); MockWebServer mockWebServer = new MockWebServer(); mockWebServer.enqueue( @@ -128,4 +125,22 @@ public class DefaultHttpDataSourceTest { assertThat(exception.responseCode).isEqualTo(404); assertThat(exception.responseBody).isEqualTo(TestUtil.createByteArray(1, 2, 3)); } + + @Test + public void factory_setRequestPropertyAfterCreation_setsCorrectHeaders() throws Exception { + MockWebServer mockWebServer = new MockWebServer(); + mockWebServer.enqueue(new MockResponse()); + DataSpec dataSpec = + new DataSpec.Builder().setUri(mockWebServer.url("/test-path").toString()).build(); + DefaultHttpDataSource.Factory factory = new DefaultHttpDataSource.Factory(); + HttpDataSource dataSource = factory.createDataSource(); + + Map defaultRequestProperties = new HashMap<>(); + defaultRequestProperties.put("0", "afterCreation"); + factory.setDefaultRequestProperties(defaultRequestProperties); + dataSource.open(dataSpec); + + Headers headers = mockWebServer.takeRequest(10, SECONDS).getHeaders(); + assertThat(headers.get("0")).isEqualTo("afterCreation"); + } } diff --git a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java index f81c866019..23ea5630ae 100644 --- a/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java +++ b/testutils/src/main/java/com/google/android/exoplayer2/testutil/FakeMediaChunk.java @@ -26,7 +26,8 @@ import com.google.android.exoplayer2.upstream.DefaultHttpDataSource; /** Fake {@link MediaChunk}. */ public final class FakeMediaChunk extends MediaChunk { - private static final DataSource DATA_SOURCE = new DefaultHttpDataSource("TEST_AGENT"); + private static final DataSource DATA_SOURCE = + new DefaultHttpDataSource.Factory().setUserAgent("TEST_AGENT").createDataSource(); /** * Creates a fake media chunk.