Base available on content length for http streams.

Fixes #392.
bumptech · Apr 6, 2015 · 64be9d3 · 64be9d3
1 parent 52c7e92
commit 64be9d3
Show file tree

Hide file tree

Showing 5 changed files with 221 additions and 4 deletions.
diff --git a/...ation/okhttp/src/main/java/com/bumptech/glide/integration/okhttp/OkHttpStreamFetcher.java b/...ation/okhttp/src/main/java/com/bumptech/glide/integration/okhttp/OkHttpStreamFetcher.java
@@ -3,6 +3,7 @@
 import com.bumptech.glide.Priority;
 import com.bumptech.glide.load.data.DataFetcher;
 import com.bumptech.glide.load.model.GlideUrl;
+import com.bumptech.glide.util.ContentLengthInputStream;
 import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.Request;
 import com.squareup.okhttp.Response;
@@ -16,6 +17,7 @@
  * Fetches an {@link InputStream} using the okhttp library.
  */
 public class OkHttpStreamFetcher implements DataFetcher<InputStream> {
+    private static final String CONTENT_LENGTH_HEADER = "Content-Length";
     private final OkHttpClient client;
     private final GlideUrl url;
     private InputStream stream;
@@ -40,7 +42,9 @@ public InputStream loadData(Priority priority) throws Exception {
         if (!response.isSuccessful()) {
             throw new IOException("Request failed with code: " + response.code());
         }
-        stream = responseBody.byteStream();
+
+        String contentLength = response.header(CONTENT_LENGTH_HEADER);
+        stream = ContentLengthInputStream.obtain(responseBody.byteStream(), contentLength);
         return stream;
     }
 

diff --git a/library/src/androidTest/java/com/bumptech/glide/load/data/HttpUrlFetcherTest.java b/library/src/androidTest/java/com/bumptech/glide/load/data/HttpUrlFetcherTest.java
@@ -15,16 +15,18 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
 import org.mockito.InOrder;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
 
-@RunWith(JUnit4.class)
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE, emulateSdk = 18)
 public class HttpUrlFetcherTest {
     private HttpURLConnection urlConnection;
     private HttpUrlFetcher fetcher;

diff --git a/library/src/androidTest/java/com/bumptech/glide/util/ContentLengthInputStreamTest.java b/library/src/androidTest/java/com/bumptech/glide/util/ContentLengthInputStreamTest.java
@@ -0,0 +1,134 @@
+package com.bumptech.glide.util;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE, emulateSdk = 18)
+public class ContentLengthInputStreamTest {
+  @Mock InputStream wrapped;
+
+  @Before
+  public void setUp() {
+    MockitoAnnotations.initMocks(this);
+  }
+
+  @Test
+  public void testAvailable_withZeroReadsAndValidContentLength_returnsContentLength()
+      throws IOException {
+    int value = 123356;
+    InputStream is = ContentLengthInputStream.obtain(wrapped, String.valueOf(value));
+
+    assertThat(is.available()).isEqualTo(value);
+  }
+
+  @Test
+  public void testAvailable_withNullContentLength_returnsWrappedAvailable()
+      throws IOException {
+    InputStream is = ContentLengthInputStream.obtain(wrapped, null /*contentLengthHeader*/);
+    int expected = 1234;
+    when(wrapped.available()).thenReturn(expected);
+
+    assertThat(is.available()).isEqualTo(expected);
+  }
+
+  @Test
+  public void testAvailable_withInvalidContentLength_returnsWrappedAvailable() throws IOException {
+    InputStream is = ContentLengthInputStream.obtain(wrapped, "invalid_length");
+    int expected = 567;
+    when(wrapped.available()).thenReturn(expected);
+
+    assertThat(is.available()).isEqualTo(expected);
+  }
+
+  @Test
+  public void testAvailable_withRead_returnsContentLengthOffsetByRead() throws IOException {
+    int contentLength = 999;
+    InputStream is = ContentLengthInputStream.obtain(wrapped, String.valueOf(contentLength));
+    when(wrapped.read()).thenReturn(1);
+
+    assertThat(is.read()).isEqualTo(1);
+    assertThat(is.available()).isEqualTo(contentLength - 1);
+  }
+
+  @Test
+  public void testAvailable_handlesReadValueOfZero() throws IOException {
+    int contentLength = 999;
+    InputStream is = ContentLengthInputStream.obtain(wrapped, String.valueOf(contentLength));
+    when(wrapped.read()).thenReturn(0);
+
+    assertThat(is.read()).isEqualTo(0);
+    assertThat(is.available()).isEqualTo(contentLength);
+  }
+
+  @Test
+  public void testAvailable_withReadBytes_returnsContentLengthOffsetByNumberOfBytes()
+      throws IOException {
+    int contentLength = 678;
+    InputStream is = ContentLengthInputStream.obtain(wrapped, String.valueOf(contentLength));
+    int read = 100;
+    when(wrapped.read(any(byte[].class), anyInt(), anyInt())).thenReturn(read);
+
+    assertThat(is.read(new byte[500], 0, 0)).isEqualTo(read);
+    assertThat(is.available()).isEqualTo(contentLength - read);
+  }
+
+  @Test
+  public void testRead_whenReturnsLessThanZeroWithoutReadingAllContent_throwsIOException()
+      throws IOException {
+    int contentLength = 1;
+    InputStream is = ContentLengthInputStream.obtain(wrapped, String.valueOf(contentLength));
+    when(wrapped.read()).thenReturn(-1);
+
+    try {
+      is.read();
+      fail("Failed to throw expected exception");
+    } catch (IOException e) {
+      // Expected.
+    }
+  }
+
+  @Test
+  public void testReadBytes_whenReturnsLessThanZeroWithoutReadingAllContent_throwsIOException()
+      throws IOException {
+    int contentLength = 2;
+    InputStream is = ContentLengthInputStream.obtain(wrapped, String.valueOf(contentLength));
+    when(wrapped.read(any(byte[].class), anyInt(), anyInt())).thenReturn(-1);
+
+    try {
+      is.read(new byte[10], 0, 0);
+      fail("Failed to throw expected exception");
+    } catch (IOException e) {
+      // Expected.
+    }
+  }
+
+  @Test
+  public void testRead_whenReturnsLessThanZeroWithInvalidLength_doesNotThrow() throws IOException {
+    InputStream is = ContentLengthInputStream.obtain(wrapped, "invalid_length");
+    when(wrapped.read()).thenReturn(-1);
+    is.read();
+  }
+
+  @Test
+  public void testReadBytes_whenReturnsLessThanZeroWithInvalidLength_doesNotThrow()
+      throws IOException {
+    InputStream is = ContentLengthInputStream.obtain(wrapped, "invalid_length");
+    when(wrapped.read(any(byte[].class), anyInt(), anyInt())).thenReturn(-1);
+    is.read(new byte[10], 0, 0);
+  }
+}
diff --git a/library/src/main/java/com/bumptech/glide/load/data/HttpUrlFetcher.java b/library/src/main/java/com/bumptech/glide/load/data/HttpUrlFetcher.java
@@ -4,6 +4,7 @@
 
 import com.bumptech.glide.Priority;
 import com.bumptech.glide.load.model.GlideUrl;
+import com.bumptech.glide.util.ContentLengthInputStream;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -16,6 +17,7 @@
  * A DataFetcher that retrieves an {@link java.io.InputStream} for a Url.
  */
 public class HttpUrlFetcher implements DataFetcher<InputStream> {
+    private static final String CONTENT_LENGTH = "Content-Length";
     private static final int MAXIMUM_REDIRECTS = 5;
     private static final HttpUrlConnectionFactory DEFAULT_CONNECTION_FACTORY = new DefaultHttpUrlConnectionFactory();
 
@@ -72,7 +74,8 @@ private InputStream loadDataWithRedirects(URL url, int redirects, URL lastUrl, M
         }
         final int statusCode = urlConnection.getResponseCode();
         if (statusCode / 100 == 2) {
-            stream = urlConnection.getInputStream();
+            String contentLength = urlConnection.getHeaderField(CONTENT_LENGTH);
+            stream = ContentLengthInputStream.obtain(urlConnection.getInputStream(), contentLength);
             return stream;
         } else if (statusCode / 100 == 3) {
             String redirectUrlString = urlConnection.getHeaderField("Location");

diff --git a/library/src/main/java/com/bumptech/glide/util/ContentLengthInputStream.java b/library/src/main/java/com/bumptech/glide/util/ContentLengthInputStream.java
@@ -0,0 +1,74 @@
+package com.bumptech.glide.util;
+
+import android.text.TextUtils;
+import android.util.Log;
+
+import java.io.FilterInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Uses the content length as the basis for the return value of {@link #available()} and verifies
+ * that at least content length bytes are returned from the various read methods.
+ */
+public final class ContentLengthInputStream extends FilterInputStream {
+  private static final String TAG = "ContentLengthStream";
+  private static final int UNKNOWN = -1;
+
+  private final int contentLength;
+  private int readSoFar;
+
+  public static InputStream obtain(InputStream other, String contentLengthHeader) {
+    return new ContentLengthInputStream(other, parseContentLength(contentLengthHeader));
+  }
+
+  private static int parseContentLength(String contentLengthHeader) {
+    int result = UNKNOWN;
+    if (!TextUtils.isEmpty(contentLengthHeader)) {
+      try {
+        result = Integer.parseInt(contentLengthHeader);
+      } catch (NumberFormatException e) {
+        if (Log.isLoggable(TAG, Log.DEBUG)) {
+          Log.d(TAG, "failed to parse content length header: " + contentLengthHeader, e);
+        }
+      }
+    }
+    return result;
+  }
+
+  ContentLengthInputStream(InputStream in, int contentLength) {
+    super(in);
+    this.contentLength = contentLength;
+  }
+
+  @Override
+  public synchronized int available() throws IOException {
+    return Math.max(contentLength - readSoFar, in.available());
+ }
+
+  @Override
+  public synchronized int read() throws IOException {
+    return checkReadSoFarOrThrow(super.read());
+  }
+
+  @Override
+  public int read(byte[] buffer) throws IOException {
+    return read(buffer, 0 /*byteOffset*/, buffer.length /*byteCount*/);
+  }
+
+  @Override
+  public synchronized int read(byte[] buffer, int byteOffset, int byteCount) throws IOException {
+    return checkReadSoFarOrThrow(super.read(buffer, byteOffset, byteCount));
+  }
+
+  private int checkReadSoFarOrThrow(int read) throws IOException {
+    if (read >= 0) {
+      readSoFar += read;
+    } else if (contentLength - readSoFar > 0) {
+      throw new IOException("Failed to read all expected data"
+          + ", expected: " + contentLength
+          + ", but read: " + readSoFar);
+    }
+    return read;
+  }
+}