Niedzielski has uploaded a new change for review. https://gerrit.wikimedia.org/r/293031
Change subject: Check responses of synchronous network requests ...................................................................... Check responses of synchronous network requests A follow up to 9a21153, check the response statuses of synchronous network requests. Probably simply because synchronous requests are less common, I don't see checking the status called out explicitly in the docs for the synchronous case. However, the status is checked in many usages of the asynchronous Callback.onResponse(), and it seems surprising not to for either case. There is also mention of checking the synchronous status in this sample[0]. [0] https://github.com/square/retrofit/blob/0e4fe60/samples/src/main/java/com/example/retrofit/DeserializeErrorBody.java#L60 Change-Id: I37042a3d4a36b63988d350a90ff7d5df02bfaff4 --- M app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java M app/src/main/java/org/wikipedia/readinglist/api/ReadingListDataClient.java M app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java M app/src/main/java/org/wikipedia/server/restbase/RbPageService.java M app/src/main/java/org/wikipedia/useroption/dataclient/DefaultUserOptionDataClient.java 5 files changed, 47 insertions(+), 18 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia refs/changes/31/293031/1 diff --git a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java index 179c9aa..42e1cde 100644 --- a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java +++ b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwQueryResponse.java @@ -31,6 +31,10 @@ return query; } + public boolean success() { + return error == null && query != null; + } + @VisibleForTesting protected void setQuery(@Nullable T query) { this.query = query; diff --git a/app/src/main/java/org/wikipedia/readinglist/api/ReadingListDataClient.java b/app/src/main/java/org/wikipedia/readinglist/api/ReadingListDataClient.java index 71ad553..d2c9015 100644 --- a/app/src/main/java/org/wikipedia/readinglist/api/ReadingListDataClient.java +++ b/app/src/main/java/org/wikipedia/readinglist/api/ReadingListDataClient.java @@ -1,18 +1,19 @@ package org.wikipedia.readinglist.api; -import org.wikipedia.Site; -import org.wikipedia.readinglist.api.legacy.LegacyReadingListPageTitlesResponse; -import org.wikipedia.readinglist.api.legacy.LegacyReadingListsResponse; -import org.wikipedia.dataclient.retrofit.RetrofitFactory; - -import retrofit2.Call; -import retrofit2.http.GET; -import retrofit2.http.Query; - import android.support.annotation.NonNull; import android.support.annotation.VisibleForTesting; +import org.wikipedia.Site; +import org.wikipedia.dataclient.retrofit.RetrofitFactory; +import org.wikipedia.readinglist.api.legacy.LegacyReadingListPageTitlesResponse; +import org.wikipedia.readinglist.api.legacy.LegacyReadingListsResponse; + import java.io.IOException; + +import retrofit2.Call; +import retrofit2.Response; +import retrofit2.http.GET; +import retrofit2.http.Query; /** * Gets and posts collection related data from and to the server. @@ -35,7 +36,11 @@ * Gets the Collections of the current user. */ public LegacyReadingListsResponse getReadingLists() throws IOException { - return client.getReadingLists().execute().body(); + Response<LegacyReadingListsResponse> rsp = client.getReadingLists().execute(); + if (rsp.isSuccessful() && rsp.body().success()) { + return rsp.body(); + } + throw new IOException(rsp.message()); } /** @@ -44,7 +49,11 @@ * @param listId ID of the reading list to be retrieved */ public LegacyReadingListPageTitlesResponse getMemberPages(int listId) throws IOException { - return client.getMemberPages(listId).execute().body(); + Response<LegacyReadingListPageTitlesResponse> rsp = client.getMemberPages(listId).execute(); + if (rsp.isSuccessful() && rsp.body().success()) { + return rsp.body(); + } + throw new IOException(rsp.message()); } private interface Api { diff --git a/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java b/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java index 291fa49..c918d9d 100644 --- a/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java +++ b/app/src/main/java/org/wikipedia/server/mwapi/MwPageService.java @@ -130,9 +130,14 @@ }); } + // Synchronous @Override public MwPageCombo pageCombo(String title, boolean noImages) throws IOException { - return webService.pageCombo(title, noImages).execute().body(); + Response<MwPageCombo> rsp = webService.pageCombo(title, noImages).execute(); + if (rsp.isSuccessful() && !rsp.body().hasError()) { + return rsp.body(); + } + throw new IOException(rsp.message()); } /** diff --git a/app/src/main/java/org/wikipedia/server/restbase/RbPageService.java b/app/src/main/java/org/wikipedia/server/restbase/RbPageService.java index 0f2050a..e0ce7c7 100644 --- a/app/src/main/java/org/wikipedia/server/restbase/RbPageService.java +++ b/app/src/main/java/org/wikipedia/server/restbase/RbPageService.java @@ -146,7 +146,11 @@ @Override public RbPageCombo pageCombo(String title, boolean noImages) throws IOException { - return webService.pageCombo(title, noImages).execute().body(); + Response<RbPageCombo> rsp = webService.pageCombo(title, noImages).execute(); + if (rsp.isSuccessful() && !rsp.body().hasError()) { + return rsp.body(); + } + throw new IOException(rsp.message()); } /* Not defined in the PageService interface since the Wiktionary definition endpoint exists only diff --git a/app/src/main/java/org/wikipedia/useroption/dataclient/DefaultUserOptionDataClient.java b/app/src/main/java/org/wikipedia/useroption/dataclient/DefaultUserOptionDataClient.java index a698d82..9f93f70 100644 --- a/app/src/main/java/org/wikipedia/useroption/dataclient/DefaultUserOptionDataClient.java +++ b/app/src/main/java/org/wikipedia/useroption/dataclient/DefaultUserOptionDataClient.java @@ -17,6 +17,7 @@ import java.util.concurrent.Executor; import retrofit2.Call; +import retrofit2.Response; import retrofit2.http.Field; import retrofit2.http.FormUrlEncoded; import retrofit2.http.GET; @@ -35,16 +36,22 @@ @NonNull @Override public UserInfo get() throws IOException { - MwQueryResponse<QueryUserInfo> body = client.get().execute().body(); - if (body == null) { - throw new IOException("Received response without body."); + Response<MwQueryResponse<QueryUserInfo>> rsp = client.get().execute(); + if (rsp.isSuccessful() && rsp.body().success()) { + //noinspection ConstantConditions + return rsp.body().query().userInfo(); } - return body.query().userInfo(); + throw new IOException(rsp.message()); } @Override public void post(@NonNull UserOption option) throws IOException { - client.post(getToken(), option.key(), option.val()).execute().body().check(site); + Response<PostResponse> rsp = client.post(getToken(), option.key(), option.val()).execute(); + if (rsp.isSuccessful()) { + rsp.body().check(site); + return; + } + throw new IOException(rsp.message()); } @Override -- To view, visit https://gerrit.wikimedia.org/r/293031 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I37042a3d4a36b63988d350a90ff7d5df02bfaff4 Gerrit-PatchSet: 1 Gerrit-Project: apps/android/wikipedia Gerrit-Branch: master Gerrit-Owner: Niedzielski <sniedziel...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
