Skip to content

Commit

Permalink
Remove expectedResolvedLength from DataSourceContractTest
Browse files Browse the repository at this point in the history
Whether a resource resolves to a known length or not is more than just
a property of the resource & data source - for example if
`DataSpec.flags` contains `ALLOW_GZIP` then the length might be
unresolved. More generally, a `DataSource` could randomly return
`C.UNKNOWN_LENGTH` from `open()` 50% of the time and still fulfil the
`DataSource` interface. This makes it ~impossible to write a meaningful
assertion aroun this.

So this change relaxes the assertion slightly to more closely match the
definition of the `DataSource` interface.

We leave the `resolveToUnknownLength` toggle in
`WebServerDispatcher.Resource` because this is still useful for
simulating the case of a server that is serving a file it doesn't
know the length of.

PiperOrigin-RevId: 351124246
  • Loading branch information
icbaker committed Jan 11, 2021
1 parent 704f006 commit 01ae2b0
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ protected ImmutableList<TestResource> getTestResources() throws Exception {
.setName("simple (pipe=true)")
.setUri(TestContentProvider.buildUri(DATA_PATH, /* pipeMode= */ true))
.setExpectedBytes(completeData)
.setResolvesToUnknownLength(true)
.build());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ private static TestResource toTestResource(
.setName(name)
.setUri(Uri.parse(urlResolver.apply(resource.getPath()).toString()))
.setExpectedBytes(resource.getData())
.setResolvesToUnknownLength(resource.resolvesToUnknownLength())
.setEndOfInputExpected(!resource.resolvesToUnknownLength())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ protected ImmutableList<TestResource> getTestResources() {
.setName("local-udp-unicast-socket")
.setUri(Uri.parse("udp:https://localhost:" + findFreeUdpPort()))
.setExpectedBytes(data)
.setResolvesToUnknownLength(true)
.setEndOfInputExpected(false)
.build());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,9 @@ public void unboundedDataSpec_readEverything() throws Exception {
? Util.readToEnd(dataSource)
: Util.readExactly(dataSource, resource.getExpectedBytes().length);

assertThat(length).isEqualTo(resource.getExpectedResolvedLength());
if (length != C.LENGTH_UNSET) {
assertThat(length).isEqualTo(resource.getExpectedBytes().length);
}
assertThat(data).isEqualTo(resource.getExpectedBytes());
} finally {
dataSource.close();
Expand All @@ -120,8 +122,8 @@ public void dataSpecWithPosition_readUntilEnd() throws Exception {
? Util.readToEnd(dataSource)
: Util.readExactly(dataSource, resource.getExpectedBytes().length - 3);

if (resource.getExpectedResolvedLength() != C.LENGTH_UNSET) {
assertThat(length).isEqualTo(resource.getExpectedResolvedLength() - 3);
if (length != C.LENGTH_UNSET) {
assertThat(length).isEqualTo(resource.getExpectedBytes().length - 3);
}
byte[] expectedData =
Arrays.copyOfRange(resource.getExpectedBytes(), 3, resource.getExpectedBytes().length);
Expand Down Expand Up @@ -219,19 +221,13 @@ public static final class TestResource {
@Nullable private final String name;
private final Uri uri;
private final byte[] expectedBytes;
private final boolean resolvesToUnknownLength;
private final boolean endOfInputExpected;

private TestResource(
@Nullable String name,
Uri uri,
byte[] expectedBytes,
boolean resolvesToUnknownLength,
boolean endOfInputExpected) {
@Nullable String name, Uri uri, byte[] expectedBytes, boolean endOfInputExpected) {
this.name = name;
this.uri = uri;
this.expectedBytes = expectedBytes;
this.resolvesToUnknownLength = resolvesToUnknownLength;
this.endOfInputExpected = endOfInputExpected;
}

Expand All @@ -251,16 +247,6 @@ public byte[] getExpectedBytes() {
return expectedBytes;
}

/**
* Returns the expected resolved length of this resource.
*
* <p>This is either {@link #getExpectedBytes() getExpectedBytes().length} or {@link
* C#LENGTH_UNSET}.
*/
public long getExpectedResolvedLength() {
return resolvesToUnknownLength ? C.LENGTH_UNSET : expectedBytes.length;
}

/**
* Returns whether {@link DataSource#read} is expected to return {@link C#RESULT_END_OF_INPUT}
* after all the resource data are read.
Expand All @@ -274,7 +260,6 @@ public static final class Builder {
private @MonotonicNonNull String name;
private @MonotonicNonNull Uri uri;
private byte @MonotonicNonNull [] expectedBytes;
private boolean resolvesToUnknownLength;
private boolean endOfInputExpected;

/** Construct a new instance. */
Expand Down Expand Up @@ -307,16 +292,6 @@ public Builder setExpectedBytes(byte[] expectedBytes) {
return this;
}

/**
* Sets whether {@link DataSource#open(DataSpec)} is expected to return {@link C#LENGTH_UNSET}
* when passed the URI of this resource and a {@link DataSpec} with {@code length ==
* C.LENGTH_UNSET}.
*/
public Builder setResolvesToUnknownLength(boolean value) {
this.resolvesToUnknownLength = value;
return this;
}

/**
* Sets whether {@link DataSource#read} is expected to return {@link C#RESULT_END_OF_INPUT}
* after all the resource data have been read. By default, this is set to {@code true}.
Expand All @@ -328,11 +303,7 @@ public Builder setEndOfInputExpected(boolean expected) {

public TestResource build() {
return new TestResource(
name,
checkNotNull(uri),
checkNotNull(expectedBytes),
resolvesToUnknownLength,
endOfInputExpected);
name, checkNotNull(uri), checkNotNull(expectedBytes), endOfInputExpected);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ public Builder supportsRangeRequests(boolean supportsRangeRequests) {
/**
* Sets if the resource should resolve to an unknown length. Defaults to false.
*
* <p>If true, responses to unbound requests won't include a Content-Length header and
* Content-Range headers won't include the total resource length.
*
* @return this builder, for convenience.
*/
public Builder resolvesToUnknownLength(boolean resolvesToUnknownLength) {
Expand Down Expand Up @@ -133,12 +136,7 @@ public boolean supportsRangeRequests() {
return supportsRangeRequests;
}

/**
* Returns true if the server shouldn't include the resource length in header responses.
*
* <p>Responses to unbound requests won't include a Content-Length header, and Content-Range
* headers won't include the total resource length.
*/
/** Returns true if the resource should resolve to an unknown length. */
public boolean resolvesToUnknownLength() {
return resolvesToUnknownLength;
}
Expand Down

0 comments on commit 01ae2b0

Please sign in to comment.