Contract test for start-position-out-of-range case

Currently, this only asserts that *if* an exception is thrown, it
must be a position-out-of-range exception as determined by
DataSourceException.isCausedByPositionOutOfRange.

Issue: #7326
PiperOrigin-RevId: 359092124
This commit is contained in:
olly 2021-02-23 19:27:46 +00:00 committed by marcbaechinger
parent 32e3f15e8f
commit 19adc43bf1
6 changed files with 80 additions and 49 deletions

View File

@ -17,15 +17,12 @@ package com.google.android.exoplayer2.upstream;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static junit.framework.Assert.fail; import static junit.framework.Assert.fail;
import static org.junit.Assert.assertThrows;
import android.net.Uri; import android.net.Uri;
import androidx.test.core.app.ApplicationProvider; import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.testutil.TestUtil; import com.google.android.exoplayer2.testutil.TestUtil;
import com.google.android.exoplayer2.upstream.ContentDataSource.ContentDataSourceException;
import java.io.EOFException;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.util.Arrays; import java.util.Arrays;
@ -85,36 +82,6 @@ public final class ContentDataSourceTest {
} }
} }
@Test
public void read_positionPastEndOfContent_throwsEOFException() throws Exception {
Uri contentUri = TestContentProvider.buildUri(DATA_PATH, /* pipeMode= */ false);
ContentDataSource dataSource =
new ContentDataSource(ApplicationProvider.getApplicationContext());
DataSpec dataSpec = new DataSpec(contentUri, /* position= */ 1025, C.LENGTH_UNSET);
try {
ContentDataSourceException exception =
assertThrows(ContentDataSourceException.class, () -> dataSource.open(dataSpec));
assertThat(exception).hasCauseThat().isInstanceOf(EOFException.class);
} finally {
dataSource.close();
}
}
@Test
public void readPipeMode_positionPastEndOfContent_throwsEOFException() throws Exception {
Uri contentUri = TestContentProvider.buildUri(DATA_PATH, /* pipeMode= */ true);
ContentDataSource dataSource =
new ContentDataSource(ApplicationProvider.getApplicationContext());
DataSpec dataSpec = new DataSpec(contentUri, /* position= */ 1025, C.LENGTH_UNSET);
try {
ContentDataSourceException exception =
assertThrows(ContentDataSourceException.class, () -> dataSource.open(dataSpec));
assertThat(exception).hasCauseThat().isInstanceOf(EOFException.class);
} finally {
dataSource.close();
}
}
private static void assertData(int offset, int length, boolean pipeMode) throws IOException { private static void assertData(int offset, int length, boolean pipeMode) throws IOException {
Uri contentUri = TestContentProvider.buildUri(DATA_PATH, pipeMode); Uri contentUri = TestContentProvider.buildUri(DATA_PATH, pipeMode);
ContentDataSource dataSource = ContentDataSource dataSource =
@ -130,5 +97,4 @@ public final class ContentDataSourceTest {
dataSource.close(); dataSource.close();
} }
} }
} }

View File

@ -71,7 +71,7 @@ public final class AssetDataSource extends BaseDataSource {
if (skipped < dataSpec.position) { if (skipped < dataSpec.position) {
// assetManager.open() returns an AssetInputStream, whose skip() implementation only skips // assetManager.open() returns an AssetInputStream, whose skip() implementation only skips
// fewer bytes than requested if the skip is beyond the end of the asset's data. // fewer bytes than requested if the skip is beyond the end of the asset's data.
throw new EOFException(); throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE);
} }
if (dataSpec.length != C.LENGTH_UNSET) { if (dataSpec.length != C.LENGTH_UNSET) {
bytesRemaining = dataSpec.length; bytesRemaining = dataSpec.length;

View File

@ -80,7 +80,7 @@ public final class ContentDataSource extends BaseDataSource {
if (skipped != dataSpec.position) { if (skipped != dataSpec.position) {
// We expect the skip to be satisfied in full. If it isn't then we're probably trying to // We expect the skip to be satisfied in full. If it isn't then we're probably trying to
// skip beyond the end of the data. // skip beyond the end of the data.
throw new EOFException(); throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE);
} }
if (dataSpec.length != C.LENGTH_UNSET) { if (dataSpec.length != C.LENGTH_UNSET) {
bytesRemaining = dataSpec.length; bytesRemaining = dataSpec.length;
@ -96,13 +96,13 @@ public final class ContentDataSource extends BaseDataSource {
} else { } else {
bytesRemaining = channelSize - channel.position(); bytesRemaining = channelSize - channel.position();
if (bytesRemaining < 0) { if (bytesRemaining < 0) {
throw new EOFException(); throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE);
} }
} }
} else { } else {
bytesRemaining = assetFileDescriptorLength - skipped; bytesRemaining = assetFileDescriptorLength - skipped;
if (bytesRemaining < 0) { if (bytesRemaining < 0) {
throw new EOFException(); throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE);
} }
} }
} }

View File

@ -23,7 +23,6 @@ import android.text.TextUtils;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import java.io.EOFException;
import java.io.FileNotFoundException; import java.io.FileNotFoundException;
import java.io.IOException; import java.io.IOException;
import java.io.RandomAccessFile; import java.io.RandomAccessFile;
@ -91,7 +90,7 @@ public final class FileDataSource extends BaseDataSource {
bytesRemaining = dataSpec.length == C.LENGTH_UNSET ? file.length() - dataSpec.position bytesRemaining = dataSpec.length == C.LENGTH_UNSET ? file.length() - dataSpec.position
: dataSpec.length; : dataSpec.length;
if (bytesRemaining < 0) { if (bytesRemaining < 0) {
throw new EOFException(); throw new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE);
} }
} catch (IOException e) { } catch (IOException e) {
throw new FileDataSourceException(e); throw new FileDataSourceException(e);

View File

@ -88,6 +88,16 @@ public class UdpDataSourceContractTest extends DataSourceContractTest {
@Override @Override
public void dataSpecWithPositionAndLength_readExpectedRange() {} public void dataSpecWithPositionAndLength_readExpectedRange() {}
@Test
@Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]")
@Override
public void dataSpecWithPositionEqualToLength_throwsPositionOutOfRangeException() {}
@Test
@Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]")
@Override
public void dataSpecWithPositionOutOfRange_throwsPositionOutOfRangeException() {}
@Test @Test
@Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]") @Ignore("UdpDataSource doesn't support DataSpec's position or length [internal: b/175856954]")
@Override @Override

View File

@ -28,6 +28,7 @@ import androidx.annotation.Nullable;
import androidx.annotation.RequiresApi; import androidx.annotation.RequiresApi;
import com.google.android.exoplayer2.C; import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.DataSource;
import com.google.android.exoplayer2.upstream.DataSourceException;
import com.google.android.exoplayer2.upstream.DataSpec; import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.upstream.TransferListener;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
@ -215,7 +216,58 @@ public abstract class DataSourceContractTest {
@Test @Test
public void dataSpecWithPositionEqualToLength_throwsPositionOutOfRangeException() public void dataSpecWithPositionEqualToLength_throwsPositionOutOfRangeException()
throws Exception { throws Exception {
// TODO: Implement this. ImmutableList<TestResource> resources = getTestResources();
Assertions.checkArgument(!resources.isEmpty(), "Must provide at least one test resource.");
for (int i = 0; i < resources.size(); i++) {
additionalFailureInfo.setInfo(getFailureLabel(resources, i));
TestResource resource = resources.get(i);
int resourceLength = resource.getExpectedBytes().length;
DataSource dataSource = createDataSource();
DataSpec dataSpec =
new DataSpec.Builder().setUri(resource.getUri()).setPosition(resourceLength).build();
try {
dataSource.open(dataSpec);
// TODO: For any cases excluded from the requirement that a position-out-of-range exception
// is thrown, decide what the allowed behavior should be for the first read, and assert it.
} catch (IOException e) {
// TODO: Decide whether to assert that a position-out-of-range exception must or must not be
// thrown (with exclusions if necessary), rather than just asserting it must be a
// position-out-of-range exception *if* one is thrown at all.
assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
} finally {
dataSource.close();
}
additionalFailureInfo.setInfo(null);
}
}
@Test
public void dataSpecWithPositionOutOfRange_throwsPositionOutOfRangeException() throws Exception {
ImmutableList<TestResource> resources = getTestResources();
Assertions.checkArgument(!resources.isEmpty(), "Must provide at least one test resource.");
for (int i = 0; i < resources.size(); i++) {
additionalFailureInfo.setInfo(getFailureLabel(resources, i));
TestResource resource = resources.get(i);
int resourceLength = resource.getExpectedBytes().length;
DataSource dataSource = createDataSource();
DataSpec dataSpec =
new DataSpec.Builder().setUri(resource.getUri()).setPosition(resourceLength + 1).build();
try {
dataSource.open(dataSpec);
// TODO: For any cases excluded from the requirement that a position-out-of-range exception
// is thrown, decide what the allowed behavior should be for the first read, and assert it.
} catch (IOException e) {
// TODO: Decide whether to assert that a position-out-of-range exception must or must not be
// thrown (with exclusions if necessary), rather than just asserting it must be a
// position-out-of-range exception *if* one is thrown at all.
assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isTrue();
} finally {
dataSource.close();
}
additionalFailureInfo.setInfo(null);
}
} }
@Test @Test
@ -228,18 +280,18 @@ public abstract class DataSourceContractTest {
TestResource resource = resources.get(i); TestResource resource = resources.get(i);
int resourceLength = resource.getExpectedBytes().length; int resourceLength = resource.getExpectedBytes().length;
DataSource dataSource = createDataSource(); DataSource dataSource = createDataSource();
DataSpec dataSpec =
new DataSpec.Builder()
.setUri(resource.getUri())
.setPosition(resourceLength - 1)
.setLength(2)
.build();
try { try {
long length = long length = dataSource.open(dataSpec);
dataSource.open(
new DataSpec.Builder()
.setUri(resource.getUri())
.setPosition(resourceLength - 1)
.setLength(2)
.build());
byte[] data = Util.readExactly(dataSource, /* length= */ 1); byte[] data = Util.readExactly(dataSource, /* length= */ 1);
// TODO: Decide what the allowed behavior should be for the next read, and assert it. // TODO: Decide what the allowed behavior should be for the next read, and assert it.
// The DataSpec.open contract requires the returned length to equal the length in the // The DataSource.open() contract requires the returned length to equal the length in the
// DataSpec if set. This is true even though the DataSource implementation may know that // DataSpec if set. This is true even though the DataSource implementation may know that
// fewer bytes will be read in this case. // fewer bytes will be read in this case.
assertThat(length).isEqualTo(2); assertThat(length).isEqualTo(2);
@ -360,6 +412,10 @@ public abstract class DataSourceContractTest {
} }
} }
// TODO: This works around [Internal ref: b/174231044]. Remove when possible.
@Test
public void emptyTest() {}
/** Build a label to make it clear which resource caused a given test failure. */ /** Build a label to make it clear which resource caused a given test failure. */
private static String getFailureLabel(List<TestResource> resources, int i) { private static String getFailureLabel(List<TestResource> resources, int i) {
if (resources.size() == 1) { if (resources.size() == 1) {