Skip to content

Commit

Permalink
Merge pull request square#1569 from square/jw/exception
Browse files Browse the repository at this point in the history
Pass failure exception to callback and target.
  • Loading branch information
JakeWharton authored Jan 5, 2017
2 parents a038626 + 91e7e21 commit 4aeb26f
Show file tree
Hide file tree
Showing 14 changed files with 49 additions and 41 deletions.
2 changes: 1 addition & 1 deletion picasso/src/main/java/com/squareup/picasso/Action.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public RequestWeakReference(Action action, M referent, ReferenceQueue<? super M>

abstract void complete(Bitmap result, Picasso.LoadedFrom from);

abstract void error();
abstract void error(Exception e);

void cancel() {
cancelled = true;
Expand Down
4 changes: 2 additions & 2 deletions picasso/src/main/java/com/squareup/picasso/Callback.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
public interface Callback {
void onSuccess();

void onError();
void onError(Exception e);

class EmptyCallback implements Callback {

@Override public void onSuccess() {
}

@Override public void onError() {
@Override public void onError(Exception e) {
}
}
}
4 changes: 2 additions & 2 deletions picasso/src/main/java/com/squareup/picasso/FetchAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class FetchAction extends Action<Object> {
}
}

@Override void error() {
@Override void error(Exception e) {
if (callback != null) {
callback.onError();
callback.onError(e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion picasso/src/main/java/com/squareup/picasso/GetAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ class GetAction extends Action<Void> {
@Override void complete(Bitmap result, Picasso.LoadedFrom from) {
}

@Override public void error() {
@Override public void error(Exception e) {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class ImageViewAction extends Action<ImageView> {
}
}

@Override public void error() {
@Override public void error(Exception e) {
ImageView target = this.target.get();
if (target == null) {
return;
Expand All @@ -69,7 +69,7 @@ class ImageViewAction extends Action<ImageView> {
}

if (callback != null) {
callback.onError();
callback.onError(e);
}
}

Expand Down
12 changes: 6 additions & 6 deletions picasso/src/main/java/com/squareup/picasso/Picasso.java
Original file line number Diff line number Diff line change
Expand Up @@ -527,14 +527,14 @@ void complete(BitmapHunter hunter) {
LoadedFrom from = hunter.getLoadedFrom();

if (single != null) {
deliverAction(result, from, single);
deliverAction(result, from, single, exception);
}

if (hasMultiple) {
//noinspection ForLoopReplaceableByForEach
for (int i = 0, n = joined.size(); i < n; i++) {
Action join = joined.get(i);
deliverAction(result, from, join);
deliverAction(result, from, join, exception);
}
}

Expand All @@ -551,7 +551,7 @@ void resumeAction(Action action) {

if (bitmap != null) {
// Resumed action is cached, complete immediately.
deliverAction(bitmap, MEMORY, action);
deliverAction(bitmap, MEMORY, action, null);
if (loggingEnabled) {
log(OWNER_MAIN, VERB_COMPLETED, action.request.logId(), "from " + MEMORY);
}
Expand All @@ -564,7 +564,7 @@ void resumeAction(Action action) {
}
}

private void deliverAction(Bitmap result, LoadedFrom from, Action action) {
private void deliverAction(Bitmap result, LoadedFrom from, Action action, Exception e) {
if (action.isCancelled()) {
return;
}
Expand All @@ -580,9 +580,9 @@ private void deliverAction(Bitmap result, LoadedFrom from, Action action) {
log(OWNER_MAIN, VERB_COMPLETED, action.request.logId(), "from " + from);
}
} else {
action.error();
action.error(e);
if (loggingEnabled) {
log(OWNER_MAIN, VERB_ERRORED, action.request.logId());
log(OWNER_MAIN, VERB_ERRORED, action.request.logId(), e.getMessage());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ abstract class RemoteViewsAction extends Action<RemoteViewsAction.RemoteViewsTar
}
}

@Override public void error() {
@Override public void error(Exception e) {
if (errorResId != 0) {
setImageResource(errorResId);
}
if (callback != null) {
callback.onError();
callback.onError(e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion picasso/src/main/java/com/squareup/picasso/Target.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public interface Target {
* specified via {@link RequestCreator#error(android.graphics.drawable.Drawable)}
* or {@link RequestCreator#error(int)}.
*/
void onBitmapFailed(Drawable errorDrawable);
void onBitmapFailed(Exception e, Drawable errorDrawable);

/**
* Callback invoked right before your request is submitted.
Expand Down
6 changes: 3 additions & 3 deletions picasso/src/main/java/com/squareup/picasso/TargetAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ final class TargetAction extends Action<Target> {
}
}

@Override void error() {
@Override void error(Exception e) {
Target target = getTarget();
if (target != null) {
if (errorResId != 0) {
target.onBitmapFailed(picasso.context.getResources().getDrawable(errorResId));
target.onBitmapFailed(e, picasso.context.getResources().getDrawable(errorResId));
} else {
target.onBitmapFailed(errorDrawable);
target.onBitmapFailed(e, errorDrawable);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,9 +180,10 @@ public class DispatcherTest {
new FetchAction(mockPicasso(), new Request.Builder(URI_1).build(), 0, 0, pausedTag,
URI_KEY_1, callback);
dispatcher.performSubmit(fetchAction, false);
fetchAction.error();
Exception e = new RuntimeException();
fetchAction.error(e);

verify(callback).onError();
verify(callback).onError(e);
}

@Test public void performCancelWithFetchActionWithCallback() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void returnsIfTargetIsNullOnError() throws Exception {
new ImageViewAction(picasso, target, null, 0, 0, 0, null, URI_KEY_1, null,
callback, false);
request.target.clear();
request.error();
request.error(new RuntimeException());
verifyZeroInteractions(target);
verifyZeroInteractions(callback);
}
Expand Down Expand Up @@ -102,9 +102,10 @@ public void invokesTargetAndCallbackErrorIfTargetIsNotNullWithErrorResourceId()
ImageViewAction request =
new ImageViewAction(mock, target, null, 0, 0, RESOURCE_ID_1, null, null, null,
callback, false);
request.error();
Exception e = new RuntimeException();
request.error(e);
verify(target).setImageResource(RESOURCE_ID_1);
verify(callback).onError();
verify(callback).onError(e);
}

@Test
Expand All @@ -115,9 +116,10 @@ public void invokesErrorIfTargetIsNotNullWithErrorResourceId() throws Exception
ImageViewAction request =
new ImageViewAction(mock, target, null, 0, 0, RESOURCE_ID_1, null, null, null,
callback, false);
request.error();
Exception e = new RuntimeException();
request.error(e);
verify(target).setImageResource(RESOURCE_ID_1);
verify(callback).onError();
verify(callback).onError(e);
}

@Test
Expand All @@ -129,9 +131,10 @@ public void invokesErrorIfTargetIsNotNullWithErrorDrawable() throws Exception {
ImageViewAction request =
new ImageViewAction(mock, target, null, 0, 0, 0, errorDrawable, URI_KEY_1, null,
callback, false);
request.error();
Exception e = new RuntimeException();
request.error(e);
verify(target).setImageDrawable(errorDrawable);
verify(callback).onError();
verify(callback).onError(e);
}

@Test
Expand All @@ -155,7 +158,7 @@ public void stopPlaceholderAnimationOnError() throws Exception {
ImageViewAction request =
new ImageViewAction(picasso, target, null, 0, 0, 0, null, URI_KEY_1, null,
null, false);
request.error();
request.error(new RuntimeException());
verify(placeholder).stop();
}
}
4 changes: 2 additions & 2 deletions picasso/src/test/java/com/squareup/picasso/PicassoTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ public class PicassoTest {
when(hunter.getException()).thenReturn(exception);
when(hunter.getActions()).thenReturn(Arrays.asList(action1, action2));
picasso.complete(hunter);
verify(action1).error();
verify(action2, never()).error();
verify(action1).error(exception);
verify(action2, never()).error(exception);
verify(listener).onImageLoadFailed(picasso, URI_1, exception);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,19 @@
@Test public void errorWithNoResourceIsNoop() throws Exception {
Callback callback = mockCallback();
RemoteViewsAction action = createAction(callback);
action.error();
Exception e = new RuntimeException();
action.error(e);
verifyZeroInteractions(remoteViews);
verify(callback).onError();
verify(callback).onError(e);
}

@Test public void errorWithResourceSetsResource() throws Exception {
Callback callback = mockCallback();
RemoteViewsAction action = createAction(1, callback);
action.error();
Exception e = new RuntimeException();
action.error(e);
verify(remoteViews).setImageViewResource(1, 1);
verify(callback).onError();
verify(callback).onError(e);
}

@Test public void clearsCallbackOnCancel() throws Exception {
Expand Down
12 changes: 7 additions & 5 deletions picasso/src/test/java/com/squareup/picasso/TargetActionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ public void invokesOnBitmapFailedIfTargetIsNotNullWithErrorDrawable() throws Exc
TargetAction request =
new TargetAction(mock(Picasso.class), target, null, 0, 0, errorDrawable, URI_KEY_1, null,
0);
request.error();
verify(target).onBitmapFailed(errorDrawable);
Exception e = new RuntimeException();
request.error(e);
verify(target).onBitmapFailed(e, errorDrawable);
}

@Test
Expand All @@ -80,8 +81,9 @@ public void invokesOnBitmapFailedIfTargetIsNotNullWithErrorResourceId() throws E

when(context.getResources()).thenReturn(res);
when(res.getDrawable(RESOURCE_ID_1)).thenReturn(errorDrawable);
request.error();
verify(target).onBitmapFailed(errorDrawable);
Exception e = new RuntimeException();
request.error(e);
verify(target).onBitmapFailed(e, errorDrawable);
}

@Test public void recyclingInSuccessThrowsException() {
Expand All @@ -90,7 +92,7 @@ public void invokesOnBitmapFailedIfTargetIsNotNullWithErrorResourceId() throws E
bitmap.recycle();
}

@Override public void onBitmapFailed(Drawable errorDrawable) {
@Override public void onBitmapFailed(Exception e, Drawable errorDrawable) {
throw new AssertionError();
}

Expand Down

0 comments on commit 4aeb26f

Please sign in to comment.