Merge pull request square#363 from square/dimitris/responsex

Non 200 responses now throw exception.

Author: JakeWharton

picasso/src/main/java/com/squareup/picasso/BitmapHunter.java

@@ -89,6 +89,9 @@ protected void setExifRotation(int exifRotation) {
       } else {
         dispatcher.dispatchComplete(this);
       }
+    } catch (Downloader.ResponseException e) {
+      exception = e;
+      dispatcher.dispatchFailed(this);
     } catch (IOException e) {
       exception = e;
       dispatcher.dispatchRetry(this);

picasso/src/main/java/com/squareup/picasso/ContactsPhotoBitmapHunter.java

@@ -22,7 +22,6 @@
 import android.graphics.BitmapFactory;
 import android.net.Uri;
 import android.provider.ContactsContract;
-
 import java.io.IOException;
 import java.io.InputStream;
 

picasso/src/main/java/com/squareup/picasso/Downloader.java

@@ -35,6 +35,13 @@ public interface Downloader {
    */
   Response load(Uri uri, boolean localCacheOnly) throws IOException;
 
+  /** Thrown for non 200 responses. */
+  class ResponseException extends IOException {
+    public ResponseException(String message) {
+      super(message);
+    }
+  }
+
   /** Response stream or bitmap and info. */
   class Response {
     final InputStream stream;

picasso/src/main/java/com/squareup/picasso/OkHttpDownloader.java

@@ -105,7 +105,7 @@ protected OkHttpClient getClient() {
     int responseCode = connection.getResponseCode();
     if (responseCode >= 300) {
       connection.disconnect();
-      return null;
+      throw new ResponseException(responseCode + " " + connection.getResponseMessage());
     }
 
     String responseSource = connection.getHeaderField(RESPONSE_SOURCE_OKHTTP);

picasso/src/main/java/com/squareup/picasso/Picasso.java

@@ -32,9 +32,9 @@
 import java.util.concurrent.ExecutorService;
 
 import static android.os.Process.THREAD_PRIORITY_BACKGROUND;
+import static com.squareup.picasso.Action.RequestWeakReference;
 import static com.squareup.picasso.Dispatcher.HUNTER_BATCH_COMPLETE;
 import static com.squareup.picasso.Dispatcher.REQUEST_GCED;
-import static com.squareup.picasso.Action.RequestWeakReference;
 import static com.squareup.picasso.Utils.THREAD_PREFIX;
 
 /**

picasso/src/main/java/com/squareup/picasso/UrlConnectionDownloader.java

@@ -64,7 +64,7 @@ protected HttpURLConnection openConnection(Uri path) throws IOException {
     int responseCode = connection.getResponseCode();
     if (responseCode >= 300) {
       connection.disconnect();
-      return null;
+      throw new ResponseException(responseCode + " " + connection.getResponseMessage());
     }
 
     boolean fromCache = parseResponseSourceHeader(connection.getHeaderField(RESPONSE_SOURCE));

picasso/src/main/java/com/squareup/picasso/Utils.java

@@ -25,7 +25,6 @@
 import android.os.Process;
 import android.os.StatFs;
 import android.provider.Settings;
-
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;

picasso/src/test/java/com/squareup/picasso/BitmapHunterTest.java

@@ -100,6 +100,14 @@ public class BitmapHunterTest {
     verify(dispatcher).dispatchFailed(hunter);
   }
 
+  @Test public void responseExcpetionDispatchFailed() throws Exception {
+    Action action = mockAction(URI_KEY_1, URI_1);
+    BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action, null,
+        new Downloader.ResponseException("Test"));
+    hunter.run();
+    verify(dispatcher).dispatchFailed(hunter);
+  }
+
   @Test public void outOfMemoryDispatchFailed() throws Exception {
     when(stats.createSnapshot()).thenReturn(mock(StatsSnapshot.class));
 
@@ -119,8 +127,8 @@ public class BitmapHunterTest {
 
   @Test public void runWithIoExceptionDispatchRetry() throws Exception {
     Action action = mockAction(URI_KEY_1, URI_1);
-    BitmapHunter hunter =
-        new TestableBitmapHunter(picasso, dispatcher, cache, stats, action, null, true);
+    BitmapHunter hunter = new TestableBitmapHunter(picasso, dispatcher, cache, stats, action, null,
+        new IOException());
     hunter.run();
     verify(dispatcher).dispatchRetry(hunter);
   }
@@ -440,7 +448,7 @@ public class BitmapHunterTest {
 
   private static class TestableBitmapHunter extends BitmapHunter {
     private final Bitmap result;
-    private final boolean throwException;
+    private final IOException exception;
 
     TestableBitmapHunter(Picasso picasso, Dispatcher dispatcher, Cache cache, Stats stats,
         Action action) {
@@ -449,19 +457,19 @@ private static class TestableBitmapHunter extends BitmapHunter {
 
     TestableBitmapHunter(Picasso picasso, Dispatcher dispatcher, Cache cache, Stats stats,
         Action action, Bitmap result) {
-      this(picasso, dispatcher, cache, stats, action, result, false);
+      this(picasso, dispatcher, cache, stats, action, result, null);
     }
 
     TestableBitmapHunter(Picasso picasso, Dispatcher dispatcher, Cache cache, Stats stats,
-        Action action, Bitmap result, boolean throwException) {
+        Action action, Bitmap result, IOException exception) {
       super(picasso, dispatcher, cache, stats, action);
       this.result = result;
-      this.throwException = throwException;
+      this.exception = exception;
     }
 
     @Override Bitmap decode(Request data) throws IOException {
-      if (throwException) {
-        throw new IOException("Failed.");
+      if (exception != null) {
+        throw exception;
       }
       return result;
     }

picasso/src/test/java/com/squareup/picasso/OkHttpDownloaderTest.java

@@ -33,6 +33,7 @@
 import static com.squareup.picasso.OkHttpDownloader.RESPONSE_SOURCE_ANDROID;
 import static com.squareup.picasso.OkHttpDownloader.RESPONSE_SOURCE_OKHTTP;
 import static org.fest.assertions.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
 
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
@@ -58,16 +59,6 @@ public class OkHttpDownloaderTest {
     server.shutdown();
   }
 
-  @Test public void nonTwoHundredReturnsNull() throws Exception {
-    server.enqueue(new MockResponse().setResponseCode(302));
-    server.enqueue(new MockResponse().setResponseCode(404));
-    server.enqueue(new MockResponse().setResponseCode(500));
-
-    assertThat(loader.load(URL, false)).isNull();
-    assertThat(loader.load(URL, false)).isNull();
-    assertThat(loader.load(URL, false)).isNull();
-  }
-
   @Test public void allowExpiredSetsCacheControl() throws Exception {
     server.enqueue(new MockResponse());
     loader.load(URL, false);
@@ -94,4 +85,14 @@ public class OkHttpDownloaderTest {
     Downloader.Response response3 = loader.load(URL, true);
     assertThat(response3.cached).isTrue();
   }
+
+  @Test public void throwsResponseException() throws Exception {
+    server.enqueue(new MockResponse().setStatus("HTTP/1.1 401 Not Authorized"));
+    try {
+      loader.load(URL, false);
+      fail("Expected ResponseException.");
+    } catch (Downloader.ResponseException e) {
+      assertThat(e).hasMessage("401 Not Authorized");
+    }
+  }
 }

picasso/src/test/java/com/squareup/picasso/UrlConnectionDownloaderTest.java

@@ -34,6 +34,7 @@
 import static android.os.Build.VERSION_CODES.ICE_CREAM_SANDWICH;
 import static com.squareup.picasso.UrlConnectionDownloader.RESPONSE_SOURCE;
 import static org.fest.assertions.api.Assertions.assertThat;
+import static org.junit.Assert.fail;
 
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
@@ -59,16 +60,6 @@ public class UrlConnectionDownloaderTest {
     server.shutdown();
   }
 
-  @Test public void nonTwoHundredReturnsNull() throws Exception {
-    server.enqueue(new MockResponse().setResponseCode(302));
-    server.enqueue(new MockResponse().setResponseCode(404));
-    server.enqueue(new MockResponse().setResponseCode(500));
-
-    assertThat(loader.load(URL, false)).isNull();
-    assertThat(loader.load(URL, false)).isNull();
-    assertThat(loader.load(URL, false)).isNull();
-  }
-
   @Config(reportSdk = ICE_CREAM_SANDWICH)
   @Test public void cacheOnlyInstalledOnce() throws Exception {
     UrlConnectionDownloader.cache = null;
@@ -117,4 +108,14 @@ public class UrlConnectionDownloaderTest {
     Downloader.Response response2 = loader.load(URL, true);
     assertThat(response2.cached).isTrue();
   }
+
+  @Test public void throwsResponseException() throws Exception {
+    server.enqueue(new MockResponse().setStatus("HTTP/1.1 401 Not Authorized"));
+    try {
+      loader.load(URL, false);
+      fail("Expected ResponseException.");
+    } catch (Downloader.ResponseException e) {
+      assertThat(e).hasMessage("401 Not Authorized");
+    }
+  }
 }