From 26cbf9444daa3ebd50b1a091753462fdb81a0e73 Mon Sep 17 00:00:00 2001 From: ibaker Date: Thu, 24 Oct 2024 03:47:01 -0700 Subject: [PATCH] `DataSourceContractTest`: Tighten assertions around 'not found' URIs This change: 1. Updates `DataSourceContractTest` to allow multiple "not found" resources, and to include additional info (e.g. headers) on them. 2. Updates the contract test to assert that `DataSource.getUri()` returns the expected (non-null) value for "not found" resources between the failed `open()` call and a subsequent `close()` call. The `DataSource` is 'open' at this point (since it needs to be 'closed' later), so `getUri()` must return non-null. * This change also fixes some implementations to comply with this contract. It also renames some imprecisely named `opened` booleans that **don't** track whether the `DataSource` is open or not. 3. Updates the contract test assertions to enforce that `DataSource.getResponseHeaders()` returns any headers associated with the 'not found' resource. 4. Configures `HttpDataSourceTestEnv` to provide both 404 and "server not found" resources, with the former having expected headers associated with it. PiperOrigin-RevId: 689316121 (cherry picked from commit 4a406be1bf8c787c4882815560a644a37792a2fb) --- RELEASENOTES.md | 8 + .../DefaultHttpDataSourceContractTest.java | 6 +- .../HttpEngineDataSourceContractTest.java | 6 +- .../datasource/DefaultHttpDataSource.java | 21 ++- .../datasource/HttpEngineDataSource.java | 27 +-- .../cronet/CronetDataSourceContractTest.java | 5 +- .../datasource/cronet/CronetDataSource.java | 24 ++- .../datasource/okhttp/OkHttpDataSource.java | 21 ++- .../okhttp/OkHttpDataSourceContractTest.java | 6 +- .../test/utils/DataSourceContractTest.java | 161 +++++++++++++----- .../test/utils/HttpDataSourceTestEnv.java | 24 +++ .../test/utils/WebServerDispatcher.java | 5 +- 12 files changed, 224 insertions(+), 90 deletions(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 5cb14f1565..bfc338cc0f 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -21,6 +21,14 @@ This release includes the following changes since the resolved URI (as documented). Where this is different to the requested URI, tests can indicate this using the new `DataSourceContractTest.TestResource.Builder.setResolvedUri()` method. + * `DataSourceContractTest`: Assert that `DataSource.getUri()` and + `getResponseHeaders()` return their 'open' value after a failed call to + `open()` (due to a 'not found' resource) and before a subsequent + `close()` call. + * Overriding `DataSourceContractTest.getNotFoundResources()` allows + test sub-classes to provide multiple 'not found' resources, and to + provide any expected headers too. This allows to distinguish between + HTTP 404 (with headers) and "server not found" (no headers). * Text: * Fix CEA-608 subtitles in H.264 MPEG-TS streams not being output (this was broken in `1.5.0-alpha01` by diff --git a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DefaultHttpDataSourceContractTest.java b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DefaultHttpDataSourceContractTest.java index 7c9e750538..d979e7d264 100644 --- a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DefaultHttpDataSourceContractTest.java +++ b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/DefaultHttpDataSourceContractTest.java @@ -15,11 +15,11 @@ */ package androidx.media3.datasource; -import android.net.Uri; import androidx.media3.test.utils.DataSourceContractTest; import androidx.media3.test.utils.HttpDataSourceTestEnv; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; +import java.util.List; import org.junit.Rule; import org.junit.runner.RunWith; @@ -40,7 +40,7 @@ public class DefaultHttpDataSourceContractTest extends DataSourceContractTest { } @Override - protected Uri getNotFoundUri() { - return Uri.parse(httpDataSourceTestEnv.getNonexistentUrl()); + protected List getNotFoundResources() { + return httpDataSourceTestEnv.getNotFoundResources(); } } diff --git a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/HttpEngineDataSourceContractTest.java b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/HttpEngineDataSourceContractTest.java index 1a899fa957..9e4fcc2e8d 100644 --- a/libraries/datasource/src/androidTest/java/androidx/media3/datasource/HttpEngineDataSourceContractTest.java +++ b/libraries/datasource/src/androidTest/java/androidx/media3/datasource/HttpEngineDataSourceContractTest.java @@ -15,13 +15,13 @@ */ package androidx.media3.datasource; -import android.net.Uri; import android.net.http.HttpEngine; import androidx.media3.test.utils.DataSourceContractTest; import androidx.media3.test.utils.HttpDataSourceTestEnv; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; +import java.util.List; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import org.junit.After; @@ -53,7 +53,7 @@ public class HttpEngineDataSourceContractTest extends DataSourceContractTest { } @Override - protected Uri getNotFoundUri() { - return Uri.parse(httpDataSourceTestEnv.getNonexistentUrl()); + protected List getNotFoundResources() { + return httpDataSourceTestEnv.getNotFoundResources(); } } diff --git a/libraries/datasource/src/main/java/androidx/media3/datasource/DefaultHttpDataSource.java b/libraries/datasource/src/main/java/androidx/media3/datasource/DefaultHttpDataSource.java index f2152be2df..e48fbb501c 100644 --- a/libraries/datasource/src/main/java/androidx/media3/datasource/DefaultHttpDataSource.java +++ b/libraries/datasource/src/main/java/androidx/media3/datasource/DefaultHttpDataSource.java @@ -261,7 +261,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou @Nullable private DataSpec dataSpec; @Nullable private HttpURLConnection connection; @Nullable private InputStream inputStream; - private boolean opened; + private boolean transferStarted; private int responseCode; private long bytesToRead; private long bytesRead; @@ -296,7 +296,13 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou @Override @Nullable public Uri getUri() { - return connection == null ? null : Uri.parse(connection.getURL().toString()); + if (connection != null) { + return Uri.parse(connection.getURL().toString()); + } else if (dataSpec != null) { + return dataSpec.uri; + } else { + return null; + } } @UnstableApi @@ -372,7 +378,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou long documentSize = HttpUtil.getDocumentSize(connection.getHeaderField(HttpHeaders.CONTENT_RANGE)); if (dataSpec.position == documentSize) { - opened = true; + transferStarted = true; transferStarted(dataSpec); return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0; } @@ -442,7 +448,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou HttpDataSourceException.TYPE_OPEN); } - opened = true; + transferStarted = true; transferStarted(dataSpec); try { @@ -493,10 +499,12 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou } finally { inputStream = null; closeConnectionQuietly(); - if (opened) { - opened = false; + if (transferStarted) { + transferStarted = false; transferEnded(); } + connection = null; + dataSpec = null; } } @@ -787,7 +795,6 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou } catch (Exception e) { Log.e(TAG, "Unexpected error while disconnecting", e); } - connection = null; } } diff --git a/libraries/datasource/src/main/java/androidx/media3/datasource/HttpEngineDataSource.java b/libraries/datasource/src/main/java/androidx/media3/datasource/HttpEngineDataSource.java index 9e6c97f032..dfcdcedc33 100644 --- a/libraries/datasource/src/main/java/androidx/media3/datasource/HttpEngineDataSource.java +++ b/libraries/datasource/src/main/java/androidx/media3/datasource/HttpEngineDataSource.java @@ -38,9 +38,6 @@ import androidx.media3.common.util.Clock; import androidx.media3.common.util.ConditionVariable; import androidx.media3.common.util.UnstableApi; import androidx.media3.common.util.Util; -import androidx.media3.datasource.HttpDataSource.CleartextNotPermittedException; -import androidx.media3.datasource.HttpDataSource.HttpDataSourceException; -import androidx.media3.datasource.HttpDataSource.InvalidResponseCodeException; import com.google.common.base.Ascii; import com.google.common.base.Predicate; import com.google.common.net.HttpHeaders; @@ -341,7 +338,7 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa private final boolean keepPostFor302Redirects; // Accessed by the calling thread only. - private boolean opened; + private boolean transferStarted; private long bytesRemaining; @Nullable private DataSpec currentDataSpec; @@ -430,14 +427,20 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa @Override @Nullable public Uri getUri() { - return responseInfo == null ? null : Uri.parse(responseInfo.getUrl()); + if (responseInfo != null) { + return Uri.parse(responseInfo.getUrl()); + } else if (currentDataSpec != null) { + return currentDataSpec.uri; + } else { + return null; + } } @UnstableApi @Override public long open(DataSpec dataSpec) throws HttpDataSourceException { Assertions.checkNotNull(dataSpec); - Assertions.checkState(!opened); + Assertions.checkState(!transferStarted); operation.close(); resetConnectTimeout(); @@ -499,7 +502,7 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa long documentSize = HttpUtil.getDocumentSize(getFirstHeader(responseHeaders, HttpHeaders.CONTENT_RANGE)); if (dataSpec.position == documentSize) { - opened = true; + transferStarted = true; transferStarted(dataSpec); return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0; } @@ -558,7 +561,7 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa bytesRemaining = dataSpec.length; } - opened = true; + transferStarted = true; transferStarted(dataSpec); skipFully(bytesToSkip, dataSpec); @@ -568,7 +571,7 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa @UnstableApi @Override public int read(byte[] buffer, int offset, int length) throws HttpDataSourceException { - Assertions.checkState(opened); + Assertions.checkState(transferStarted); if (length == 0) { return 0; @@ -639,7 +642,7 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa */ @UnstableApi public int read(ByteBuffer buffer) throws HttpDataSourceException { - Assertions.checkState(opened); + Assertions.checkState(transferStarted); if (!buffer.isDirect()) { throw new IllegalArgumentException("Passed buffer is not a direct ByteBuffer"); @@ -696,8 +699,8 @@ public final class HttpEngineDataSource extends BaseDataSource implements HttpDa responseInfo = null; exception = null; finished = false; - if (opened) { - opened = false; + if (transferStarted) { + transferStarted = false; transferEnded(); } } diff --git a/libraries/datasource_cronet/src/androidTest/java/androidx/media3/datasource/cronet/CronetDataSourceContractTest.java b/libraries/datasource_cronet/src/androidTest/java/androidx/media3/datasource/cronet/CronetDataSourceContractTest.java index 437576bef6..30fa1e24cb 100644 --- a/libraries/datasource_cronet/src/androidTest/java/androidx/media3/datasource/cronet/CronetDataSourceContractTest.java +++ b/libraries/datasource_cronet/src/androidTest/java/androidx/media3/datasource/cronet/CronetDataSourceContractTest.java @@ -15,7 +15,6 @@ */ package androidx.media3.datasource.cronet; -import android.net.Uri; import androidx.media3.datasource.DataSource; import androidx.media3.test.utils.DataSourceContractTest; import androidx.media3.test.utils.HttpDataSourceTestEnv; @@ -66,7 +65,7 @@ public class CronetDataSourceContractTest extends DataSourceContractTest { } @Override - protected Uri getNotFoundUri() { - return Uri.parse(httpDataSourceTestEnv.getNonexistentUrl()); + protected List getNotFoundResources() { + return httpDataSourceTestEnv.getNotFoundResources(); } } diff --git a/libraries/datasource_cronet/src/main/java/androidx/media3/datasource/cronet/CronetDataSource.java b/libraries/datasource_cronet/src/main/java/androidx/media3/datasource/cronet/CronetDataSource.java index 39b4e034a1..34142b7eaa 100644 --- a/libraries/datasource_cronet/src/main/java/androidx/media3/datasource/cronet/CronetDataSource.java +++ b/libraries/datasource_cronet/src/main/java/androidx/media3/datasource/cronet/CronetDataSource.java @@ -463,7 +463,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { private final boolean keepPostFor302Redirects; // Accessed by the calling thread only. - private boolean opened; + private boolean transferStarted; private long bytesRemaining; // Written from the calling thread only. currentUrlRequest.start() calls ensure writes are visible @@ -555,14 +555,20 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { @Override @Nullable public Uri getUri() { - return responseInfo == null ? null : Uri.parse(responseInfo.getUrl()); + if (responseInfo != null) { + return Uri.parse(responseInfo.getUrl()); + } else if (currentDataSpec != null) { + return currentDataSpec.uri; + } else { + return null; + } } @UnstableApi @Override public long open(DataSpec dataSpec) throws HttpDataSourceException { Assertions.checkNotNull(dataSpec); - Assertions.checkState(!opened); + Assertions.checkState(!transferStarted); operation.close(); resetConnectTimeout(); @@ -624,7 +630,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { long documentSize = HttpUtil.getDocumentSize(getFirstHeader(responseHeaders, HttpHeaders.CONTENT_RANGE)); if (dataSpec.position == documentSize) { - opened = true; + transferStarted = true; transferStarted(dataSpec); return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0; } @@ -683,7 +689,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { bytesRemaining = dataSpec.length; } - opened = true; + transferStarted = true; transferStarted(dataSpec); skipFully(bytesToSkip, dataSpec); @@ -693,7 +699,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { @UnstableApi @Override public int read(byte[] buffer, int offset, int length) throws HttpDataSourceException { - Assertions.checkState(opened); + Assertions.checkState(transferStarted); if (length == 0) { return 0; @@ -764,7 +770,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { */ @UnstableApi public int read(ByteBuffer buffer) throws HttpDataSourceException { - Assertions.checkState(opened); + Assertions.checkState(transferStarted); if (!buffer.isDirect()) { throw new IllegalArgumentException("Passed buffer is not a direct ByteBuffer"); @@ -818,8 +824,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { responseInfo = null; exception = null; finished = false; - if (opened) { - opened = false; + if (transferStarted) { + transferStarted = false; transferEnded(); } } diff --git a/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java b/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java index fa4020a41a..45d8ad706a 100644 --- a/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java +++ b/libraries/datasource_okhttp/src/main/java/androidx/media3/datasource/okhttp/OkHttpDataSource.java @@ -192,7 +192,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { @Nullable private DataSpec dataSpec; @Nullable private Response response; @Nullable private InputStream responseByteStream; - private boolean opened; + private boolean connectionEstablished; private long bytesToRead; private long bytesRead; @@ -215,7 +215,13 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { @Override @Nullable public Uri getUri() { - return response == null ? null : Uri.parse(response.request().url().toString()); + if (response != null) { + return Uri.parse(response.request().url().toString()); + } else if (dataSpec != null) { + return dataSpec.uri; + } else { + return null; + } } @UnstableApi @@ -281,7 +287,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { long documentSize = HttpUtil.getDocumentSize(response.headers().get(HttpHeaders.CONTENT_RANGE)); if (dataSpec.position == documentSize) { - opened = true; + connectionEstablished = true; transferStarted(dataSpec); return dataSpec.length != C.LENGTH_UNSET ? dataSpec.length : 0; } @@ -325,7 +331,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { bytesToRead = contentLength != -1 ? (contentLength - bytesToSkip) : C.LENGTH_UNSET; } - opened = true; + connectionEstablished = true; transferStarted(dataSpec); try { @@ -352,11 +358,13 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { @UnstableApi @Override public void close() { - if (opened) { - opened = false; + if (connectionEstablished) { + connectionEstablished = false; transferEnded(); closeConnectionQuietly(); } + response = null; + dataSpec = null; } /** Establishes a connection. */ @@ -524,7 +532,6 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { private void closeConnectionQuietly() { if (response != null) { Assertions.checkNotNull(response.body()).close(); - response = null; } responseByteStream = null; } diff --git a/libraries/datasource_okhttp/src/test/java/androidx/media3/datasource/okhttp/OkHttpDataSourceContractTest.java b/libraries/datasource_okhttp/src/test/java/androidx/media3/datasource/okhttp/OkHttpDataSourceContractTest.java index c890940c9e..5dca8634b4 100644 --- a/libraries/datasource_okhttp/src/test/java/androidx/media3/datasource/okhttp/OkHttpDataSourceContractTest.java +++ b/libraries/datasource_okhttp/src/test/java/androidx/media3/datasource/okhttp/OkHttpDataSourceContractTest.java @@ -15,12 +15,12 @@ */ package androidx.media3.datasource.okhttp; -import android.net.Uri; import androidx.media3.datasource.DataSource; import androidx.media3.test.utils.DataSourceContractTest; import androidx.media3.test.utils.HttpDataSourceTestEnv; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.common.collect.ImmutableList; +import java.util.List; import okhttp3.OkHttpClient; import org.junit.Rule; import org.junit.runner.RunWith; @@ -42,7 +42,7 @@ public class OkHttpDataSourceContractTest extends DataSourceContractTest { } @Override - protected Uri getNotFoundUri() { - return Uri.parse(httpDataSourceTestEnv.getNonexistentUrl()); + protected List getNotFoundResources() { + return httpDataSourceTestEnv.getNotFoundResources(); } } diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/DataSourceContractTest.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/DataSourceContractTest.java index 00e18ad8f0..37feb0e408 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/DataSourceContractTest.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/DataSourceContractTest.java @@ -15,7 +15,6 @@ */ package androidx.media3.test.utils; -import static androidx.media3.common.util.Assertions.checkArgument; import static androidx.media3.common.util.Assertions.checkNotNull; import static androidx.media3.common.util.Assertions.checkState; import static androidx.media3.common.util.Util.castNonNull; @@ -86,7 +85,8 @@ public abstract class DataSourceContractTest { */ @ForOverride protected DataSource createDataSource() throws Exception { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException( + "Either createDataSource or createDataSources must be implemented."); } /** @@ -97,7 +97,8 @@ public abstract class DataSourceContractTest { */ @ForOverride protected List createDataSources() throws Exception { - throw new UnsupportedOperationException(); + throw new UnsupportedOperationException( + "Either createDataSource or createDataSources must be implemented."); } /** @@ -124,7 +125,8 @@ public abstract class DataSourceContractTest { * Returns {@link TestResource} instances. * *

Each resource will be used to exercise the {@link DataSource} instance, allowing different - * behaviours to be tested. + * behaviours to be tested. Every {@link TestResource#getExpectedBytes()} must be at least 5 + * bytes. * *

If multiple resources are returned, it's recommended to disambiguate them using {@link * TestResource.Builder#setName(String)}. @@ -136,9 +138,34 @@ public abstract class DataSourceContractTest { * Returns a {@link Uri} that doesn't resolve. * *

This is used to test how a {@link DataSource} handles nonexistent data. + * + *

Only one of {@link #getNotFoundUri()} and {@link #getNotFoundResources()} should be + * implemented. */ @ForOverride - protected abstract Uri getNotFoundUri(); + protected Uri getNotFoundUri() { + throw new UnsupportedOperationException( + "Either getNotFoundUri or getNotFoundUris must be implemented."); + } + + /** + * Returns a non-empty list of {@link TestResource} that don't resolve. + * + *

This is used to test how a {@link DataSource} handles nonexistent data. Multiple entries and + * the rest of the {@link TestResource} fields can be helpful for situations where the data can be + * "not found" for different reasons. For example in HTTP, 'server not found' generally results in + * a failed HTTP connection while 'file not found' generally results in a successful connection + * with a 404 HTTP error code and some response headers, and the handling code for these two cases + * may be different (and therefore worth testing separately). + * + *

Only one of {@link #getNotFoundUri()} and {@link #getNotFoundResources()} should be + * implemented. + */ + @ForOverride + protected List getNotFoundResources() { + throw new UnsupportedOperationException( + "Either getNotFoundUri or getNotFoundUris must be implemented."); + } @Test public void unboundedDataSpec_readUntilEnd() throws Exception { @@ -457,9 +484,10 @@ public abstract class DataSourceContractTest { @Test public void resourceNotFound() throws Exception { - forAllDataSources( - dataSource -> { - assertThrows(IOException.class, () -> dataSource.open(new DataSpec(getNotFoundUri()))); + forAllDataSourcesAndNotFoundResources( + (resource, dataSource) -> { + assertThrows(IOException.class, () -> dataSource.open(new DataSpec(resource.uri))); + dataSource.close(); }); } @@ -522,8 +550,8 @@ public abstract class DataSourceContractTest { @Test public void resourceNotFound_transferListenerCallbacks() throws Exception { - forAllDataSources( - dataSource -> { + forAllDataSourcesAndNotFoundResources( + (resource, dataSource) -> { TransferListener listener = mock(TransferListener.class); dataSource.addTransferListener(listener); @Nullable DataSource callbackSource = getTransferListenerDataSource(); @@ -531,7 +559,7 @@ public abstract class DataSourceContractTest { callbackSource = dataSource; } - assertThrows(IOException.class, () -> dataSource.open(new DataSpec(getNotFoundUri()))); + assertThrows(IOException.class, () -> dataSource.open(new DataSpec(resource.uri))); // Verify onTransferInitializing() has been called exactly from DataSource.open(). verify(listener).onTransferInitializing(eq(callbackSource), any(), anyBoolean()); @@ -561,13 +589,12 @@ public abstract class DataSourceContractTest { @Test public void getUri_resourceNotFound_returnsNullIfNotOpened() throws Exception { - forAllDataSources( - dataSource -> { + forAllDataSourcesAndNotFoundResources( + (resource, dataSource) -> { assertThat(dataSource.getUri()).isNull(); - - assertThrows(IOException.class, () -> dataSource.open(new DataSpec(getNotFoundUri()))); + assertThrows(IOException.class, () -> dataSource.open(new DataSpec(resource.uri))); + assertThat(dataSource.getUri()).isEqualTo(resource.uri); dataSource.close(); - assertThat(dataSource.getUri()).isNull(); }); } @@ -653,11 +680,23 @@ public abstract class DataSourceContractTest { @Test public void getResponseHeaders_resourceNotFound_isEmptyWhileNotOpen() throws Exception { - forAllDataSources( - dataSource -> { + forAllDataSourcesAndNotFoundResources( + (resource, dataSource) -> { assertThat(dataSource.getResponseHeaders()).isEmpty(); - assertThrows(IOException.class, () -> dataSource.open(new DataSpec(getNotFoundUri()))); + assertThrows(IOException.class, () -> dataSource.open(new DataSpec(resource.uri))); + + Map> actualHeaders = dataSource.getResponseHeaders(); + for (Map.Entry> expectedHeaders : + resource.getResponseHeaders().entrySet()) { + assertWithMessage("Header values for key=%s", expectedHeaders.getKey()) + .that(actualHeaders.get(expectedHeaders.getKey())) + .isEqualTo(expectedHeaders.getValue()); + } + for (String unexpectedKey : resource.getUnexpectedResponseHeaderKeys()) { + assertThat(actualHeaders).doesNotContainKey(unexpectedKey); + } + dataSource.close(); assertThat(dataSource.getResponseHeaders()).isEmpty(); @@ -668,15 +707,14 @@ public abstract class DataSourceContractTest { void run(TestResource resource, DataSource dataSource) throws Exception; } - private interface DataSourceTest { - void run(DataSource dataSource) throws Exception; - } - private void forAllTestResourcesAndDataSources(TestResourceAndDataSourceTest test) throws Exception { ImmutableList resources = getTestResources(); Assertions.checkArgument(!resources.isEmpty(), "Must provide at least one test resource."); for (int i = 0; i < resources.size(); i++) { + checkState( + resources.get(i).expectedBytes.length >= 5, + "TestResource.expectedBytes must be at least 5 bytes"); List dataSources = createDataSourcesInternal(); for (int j = 0; j < dataSources.size(); j++) { additionalFailureInfo.setInfo(getFailureLabel(resources, i, dataSources, j)); @@ -686,19 +724,24 @@ public abstract class DataSourceContractTest { } } - private void forAllDataSources(DataSourceTest test) throws Exception { - List dataSources = createDataSourcesInternal(); - for (int i = 0; i < dataSources.size(); i++) { - additionalFailureInfo.setInfo(getDataSourceLabel(dataSources, i)); - test.run(dataSources.get(i)); - additionalFailureInfo.setInfo(null); + private void forAllDataSourcesAndNotFoundResources(TestResourceAndDataSourceTest test) + throws Exception { + List notFoundResources = getNotFoundResourcesInternal(); + for (int i = 0; i < notFoundResources.size(); i++) { + List dataSources = createDataSourcesInternal(); + for (int j = 0; j < dataSources.size(); j++) { + additionalFailureInfo.setInfo( + getNotFoundResourceLabel(notFoundResources, i, dataSources, j)); + test.run(notFoundResources.get(i), dataSources.get(j)); + additionalFailureInfo.setInfo(null); + } } } private List createDataSourcesInternal() throws Exception { try { List dataSources = createDataSources(); - checkState(!dataSources.isEmpty(), "Must provide at least on DataSource"); + checkState(!dataSources.isEmpty(), "Must provide at least one DataSource"); assertThrows(UnsupportedOperationException.class, this::createDataSource); return dataSources; } catch (UnsupportedOperationException e) { @@ -707,13 +750,50 @@ public abstract class DataSourceContractTest { } } + private List getNotFoundResourcesInternal() { + try { + List notFoundResources = getNotFoundResources(); + checkState(!notFoundResources.isEmpty(), "Must provide at least one 'not found' resource"); + assertThrows(UnsupportedOperationException.class, this::getNotFoundUri); + return notFoundResources; + } catch (UnsupportedOperationException e) { + // Expected if createDataSources is not implemented. + return ImmutableList.of( + new TestResource.Builder() + .setUri(getNotFoundUri()) + .setExpectedBytes(Util.EMPTY_BYTE_ARRAY) + .build()); + } + } + + /** + * Build a label to make it clear which not-found resource and data source caused a given test + * failure. + */ + private static String getNotFoundResourceLabel( + List resources, + int resourceIndex, + List dataSources, + int dataSourceIndex) { + return getFailureLabel("not-found", resources, resourceIndex, dataSources, dataSourceIndex); + } + /** Build a label to make it clear which resource and data source caused a given test failure. */ private static String getFailureLabel( List resources, int resourceIndex, List dataSources, int dataSourceIndex) { - String resourceLabel = getResourceLabel(resources, resourceIndex); + return getFailureLabel("resources", resources, resourceIndex, dataSources, dataSourceIndex); + } + + private static String getFailureLabel( + String resourcesType, + List resources, + int resourceIndex, + List dataSources, + int dataSourceIndex) { + String resourceLabel = getResourceLabel(resourcesType, resources, resourceIndex); String dataSourceLabel = getDataSourceLabel(dataSources, dataSourceIndex); if (resourceLabel.isEmpty()) { return dataSourceLabel; @@ -724,13 +804,14 @@ public abstract class DataSourceContractTest { } } - private static String getResourceLabel(List resources, int resourceIndex) { + private static String getResourceLabel( + String resourcesType, List resources, int resourceIndex) { if (resources.size() == 1) { return ""; } else if (resources.get(resourceIndex).getName() != null) { return "resource name: " + resources.get(resourceIndex).getName(); } else { - return String.format("resource[%s]", resourceIndex); + return String.format("%s[%s]", resourcesType, resourceIndex); } } @@ -838,11 +919,12 @@ public abstract class DataSourceContractTest { private @MonotonicNonNull Uri resolvedUri; private Map> responseHeaders; private Set unexpectedResponseHeaderKeys; - private byte @MonotonicNonNull [] expectedBytes; + private byte[] expectedBytes; public Builder() { responseHeaders = ImmutableMap.of(); unexpectedResponseHeaderKeys = ImmutableSet.of(); + expectedBytes = Util.EMPTY_BYTE_ARRAY; } /** @@ -909,15 +991,10 @@ public abstract class DataSourceContractTest { return this; } - /** - * Sets the expected contents of this resource. - * - *

Must be at least 5 bytes. - */ + /** Sets the expected contents of this resource. Defaults to an empty byte array. */ @CanIgnoreReturnValue public Builder setExpectedBytes(byte[] expectedBytes) { - checkArgument(expectedBytes.length >= 5); - this.expectedBytes = expectedBytes; + this.expectedBytes = checkNotNull(expectedBytes); return this; } @@ -928,7 +1005,7 @@ public abstract class DataSourceContractTest { resolvedUri != null ? resolvedUri : uri, ImmutableMap.copyOf(responseHeaders), ImmutableSet.copyOf(unexpectedResponseHeaderKeys), - checkNotNull(expectedBytes)); + expectedBytes); } } } diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/HttpDataSourceTestEnv.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/HttpDataSourceTestEnv.java index 9830831a69..950bd16ce6 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/HttpDataSourceTestEnv.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/HttpDataSourceTestEnv.java @@ -20,9 +20,11 @@ import static androidx.media3.test.utils.WebServerDispatcher.getRequestPath; import android.net.Uri; import androidx.media3.common.util.UnstableApi; +import androidx.media3.common.util.Util; import androidx.media3.datasource.HttpDataSource; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import com.google.common.net.HttpHeaders; @@ -119,6 +121,28 @@ public class HttpDataSourceTestEnv extends ExternalResource { .build()); } + public ImmutableList getNotFoundResources() { + return ImmutableList.of( + new DataSourceContractTest.TestResource.Builder() + .setName("404") + .setUri(Uri.parse(originServer.url("/not/a/real/path").toString())) + .setResponseHeaders( + ImmutableMap.of( + HttpHeaders.CONTENT_LENGTH, + ImmutableList.of(String.valueOf(WebServerDispatcher.NOT_FOUND_BODY.length())))) + .setExpectedBytes(Util.getUtf8Bytes(WebServerDispatcher.NOT_FOUND_BODY)) + .build(), + new DataSourceContractTest.TestResource.Builder() + .setName("no-connection") + .setUri(Uri.parse("http://not-a-real-server.test/path")) + .setUnexpectedResponseHeaderKeys(ImmutableSet.of(HttpHeaders.CONTENT_LENGTH)) + .build()); + } + + /** + * @deprecated Use {@link #getNotFoundResources()} instead. + */ + @Deprecated public String getNonexistentUrl() { return originServer.url("/not/a/real/path").toString(); } diff --git a/libraries/test_utils/src/main/java/androidx/media3/test/utils/WebServerDispatcher.java b/libraries/test_utils/src/main/java/androidx/media3/test/utils/WebServerDispatcher.java index 9c5f5581e2..8e7de0cf6b 100644 --- a/libraries/test_utils/src/main/java/androidx/media3/test/utils/WebServerDispatcher.java +++ b/libraries/test_utils/src/main/java/androidx/media3/test/utils/WebServerDispatcher.java @@ -60,6 +60,9 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; @UnstableApi public class WebServerDispatcher extends Dispatcher { + /** The body associated with a response for an unrecognized path. */ + public static final String NOT_FOUND_BODY = "Resource not found!"; + /** A resource served by {@link WebServerDispatcher}. */ public static class Resource { @@ -294,7 +297,7 @@ public class WebServerDispatcher extends Dispatcher { String requestPath = getRequestPath(request); MockResponse response = new MockResponse(); if (!resourcesByPath.containsKey(requestPath)) { - return response.setResponseCode(404); + return response.setBody(NOT_FOUND_BODY).setResponseCode(404); } Resource resource = checkNotNull(resourcesByPath.get(requestPath)); for (Map.Entry extraHeader : resource.getExtraResponseHeaders().entries()) {