jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/331014 )
Change subject: Make WikitextClient expect a MwQueryResponse<Wikitext>
......................................................................
Make WikitextClient expect a MwQueryResponse<Wikitext>
WikitextClient currently can't adequately detect and handle MediaWiki
API errors because its requests are for bare Wikitext objects with no
notion of an error response, rather than a MwQueryResponse<Wikitext>.
This changes it to use the latter.
This should fix any crashes, but the underlying cause in T152982 is
still unknown. An error can occur if the section the user requests
disappears between loading the page and clicking edit, but that doesn't
actually appear to be what's going on here.
Also, our error handling in the section editing activity is inadequate
and will need updating for whatever resolution we decide on. (T154805)
Bug: T152982
Bug: T154807
Change-Id: I1955924c8b3230a536acaa9b44218db7ca3a663b
---
M
app/src/androidTest/java/org/wikipedia/edit/preview/FetchSectionWikitextTest.java
A app/src/main/java/org/wikipedia/dataclient/mwapi/MwApiErrorException.java
M app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
M app/src/main/java/org/wikipedia/edit/preview/Wikitext.java
M app/src/main/java/org/wikipedia/edit/preview/WikitextClient.java
5 files changed, 67 insertions(+), 34 deletions(-)
Approvals:
Niedzielski: Looks good to me, approved
jenkins-bot: Verified
diff --git
a/app/src/androidTest/java/org/wikipedia/edit/preview/FetchSectionWikitextTest.java
b/app/src/androidTest/java/org/wikipedia/edit/preview/FetchSectionWikitextTest.java
index 0cd7382..0ef06f2 100644
---
a/app/src/androidTest/java/org/wikipedia/edit/preview/FetchSectionWikitextTest.java
+++
b/app/src/androidTest/java/org/wikipedia/edit/preview/FetchSectionWikitextTest.java
@@ -5,6 +5,7 @@
import org.junit.Test;
import org.wikipedia.dataclient.WikiSite;
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import org.wikipedia.page.PageTitle;
import org.wikipedia.testlib.TestLatch;
@@ -21,13 +22,13 @@
new WikitextClient().request(wiki, title, sectionId, new
WikitextClient.Callback() {
@Override
- public void success(@NonNull Call<Wikitext> call, @NonNull String
wikitext) {
+ public void success(@NonNull Call<MwQueryResponse<Wikitext>> call,
@NonNull String wikitext) {
assertNotNull(wikitext);
latch.countDown();
}
@Override
- public void failure(@NonNull Call<Wikitext> call, @NonNull
Throwable caught) {
+ public void failure(@NonNull Call<MwQueryResponse<Wikitext>> call,
@NonNull Throwable caught) {
throw new RuntimeException(caught);
}
});
diff --git
a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwApiErrorException.java
b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwApiErrorException.java
new file mode 100644
index 0000000..8b505b2
--- /dev/null
+++ b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwApiErrorException.java
@@ -0,0 +1,22 @@
+package org.wikipedia.dataclient.mwapi;
+
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+import org.wikipedia.server.mwapi.MwServiceError;
+
+public class MwApiErrorException extends Throwable {
+ @SuppressWarnings("unused") @NonNull private MwServiceError error;
+
+ public MwApiErrorException(@NonNull MwServiceError error) {
+ this.error = error;
+ }
+
+ @Nullable String getTitle() {
+ return error.getTitle();
+ }
+
+ @Override @Nullable public String getMessage() {
+ return error.getDetails();
+ }
+}
diff --git a/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
b/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
index d170fa3..a73de5d 100644
--- a/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
+++ b/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
@@ -35,6 +35,7 @@
import org.wikipedia.analytics.LoginFunnel;
import org.wikipedia.captcha.CaptchaHandler;
import org.wikipedia.captcha.CaptchaResult;
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import org.wikipedia.edit.preview.EditPreviewFragment;
import org.wikipedia.edit.preview.Wikitext;
import org.wikipedia.edit.preview.WikitextClient;
@@ -471,6 +472,10 @@
return;
}
progressDialog.dismiss();
+
+ // TODO: The view shown here states "cannot connect to
the Internet" regardless
+ // of the actual cause of failure (which could also be
an API error or malformed
+ // response). Update this to provide more specificity
and accuracy. (T154805)
ViewAnimations.crossFade(sectionText, sectionError);
sectionError.setVisibility(View.VISIBLE);
}
@@ -620,13 +625,16 @@
if (sectionWikitext == null) {
new WikitextClient().request(title.getWikiSite(), title,
sectionID, new WikitextClient.Callback() {
@Override
- public void success(@NonNull Call<Wikitext> call, @NonNull
String wikitext) {
+ public void success(@NonNull Call<MwQueryResponse<Wikitext>>
call, @NonNull String wikitext) {
sectionWikitext = wikitext;
displaySectionText();
}
@Override
- public void failure(@NonNull Call<Wikitext> call, @NonNull
Throwable throwable) {
+ public void failure(@NonNull Call<MwQueryResponse<Wikitext>>
call, @NonNull Throwable throwable) {
+ // TODO: The view shown here states "cannot connect to the
Internet" regardless
+ // of the actual cause of failure (which could also be an
API error or malformed
+ // response). Update this to provide more specificity and
accuracy. (T154805)
ViewAnimations.crossFade(sectionProgress, sectionError);
// Not sure why this is required, but without it tapping
retry hides langLinksError
// FIXME: INVESTIGATE WHY THIS HAPPENS!
diff --git a/app/src/main/java/org/wikipedia/edit/preview/Wikitext.java
b/app/src/main/java/org/wikipedia/edit/preview/Wikitext.java
index 5516217..81ad46d 100644
--- a/app/src/main/java/org/wikipedia/edit/preview/Wikitext.java
+++ b/app/src/main/java/org/wikipedia/edit/preview/Wikitext.java
@@ -1,8 +1,7 @@
package org.wikipedia.edit.preview;
-import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
-import org.wikipedia.json.annotations.Required;
import org.wikipedia.model.BaseModel;
import org.wikipedia.server.mwapi.MwApiResponsePage;
@@ -11,18 +10,20 @@
import java.util.Map;
public class Wikitext extends BaseModel {
- @SuppressWarnings("unused,NullableProblems") @NonNull @Required private
Query query;
- @NonNull protected String wikitext() {
- return query.wikitext();
- }
+
@SuppressWarnings("unused,NullableProblems,MismatchedQueryAndUpdateOfCollection")
@Nullable
+ private Map<String, MwApiResponsePage> pages;
- private static class Query {
- @SuppressWarnings("unused,NullableProblems") @NonNull private
Map<String, MwApiResponsePage> pages;
- @NonNull String wikitext() {
- Iterator<Map.Entry<String, MwApiResponsePage>> i =
pages.entrySet().iterator();
- MwApiResponsePage page = i.next().getValue();
- ArrayList<MwApiResponsePage.Revision> revisions = new
ArrayList<>(page.revisions());
- return revisions.get(0).content();
+ @Nullable String wikitext() {
+ if (pages == null) {
+ return null;
}
+ Iterator<Map.Entry<String, MwApiResponsePage>> i =
pages.entrySet().iterator();
+ MwApiResponsePage page = i.next().getValue();
+ if (page == null
+ || page.revisions() == null
+ || page.revisions().get(0) == null) {
+ return null;
+ }
+ return new ArrayList<>(page.revisions()).get(0).content();
}
}
diff --git a/app/src/main/java/org/wikipedia/edit/preview/WikitextClient.java
b/app/src/main/java/org/wikipedia/edit/preview/WikitextClient.java
index 089ef1e..9be4540 100644
--- a/app/src/main/java/org/wikipedia/edit/preview/WikitextClient.java
+++ b/app/src/main/java/org/wikipedia/edit/preview/WikitextClient.java
@@ -7,10 +7,11 @@
import com.google.gson.JsonParseException;
import org.wikipedia.dataclient.WikiSite;
+import org.wikipedia.dataclient.mwapi.MwApiErrorException;
+import org.wikipedia.dataclient.mwapi.MwQueryResponse;
import org.wikipedia.dataclient.retrofit.MwCachedService;
import org.wikipedia.dataclient.retrofit.RetrofitException;
import org.wikipedia.page.PageTitle;
-import org.wikipedia.util.log.L;
import retrofit2.Call;
import retrofit2.Response;
@@ -20,34 +21,35 @@
public class WikitextClient {
@NonNull private final MwCachedService<Service> cachedService = new
MwCachedService<>(Service.class);
- public Call<Wikitext> request(@NonNull final WikiSite wiki, @NonNull final
PageTitle title,
+ public Call<MwQueryResponse<Wikitext>> request(@NonNull final WikiSite
wiki, @NonNull final PageTitle title,
final int sectionID, @NonNull final Callback
cb) {
Service service = cachedService.service(wiki);
return request(service, title, sectionID, cb);
}
- @VisibleForTesting Call<Wikitext> request(@NonNull Service service,
@NonNull final PageTitle title,
- final int sectionID, @NonNull final Callback cb) {
- Call<Wikitext> call = service.wikitext(title.getPrefixedText(),
sectionID);
- call.enqueue(new retrofit2.Callback<Wikitext>() {
+ @VisibleForTesting Call<MwQueryResponse<Wikitext>> request(@NonNull
Service service, @NonNull final PageTitle title,
+ final int
sectionID, @NonNull final Callback cb) {
+ Call<MwQueryResponse<Wikitext>> call =
service.request(title.getPrefixedText(), sectionID);
+ call.enqueue(new retrofit2.Callback<MwQueryResponse<Wikitext>>() {
@Override
- public void onResponse(Call<Wikitext> call, Response<Wikitext>
response) {
+ public void onResponse(Call<MwQueryResponse<Wikitext>> call,
Response<MwQueryResponse<Wikitext>> response) {
if (response.isSuccessful()) {
- if (response.body() == null) {
- // TODO: remove when this is better understood.
- Throwable t = new JsonParseException("Response missing
query field");
- L.logRemoteErrorIfProd(t);
+ if (response.body().hasError()) {
+ cb.failure(call, new
MwApiErrorException(response.body().getError()));
+ return;
+ } else if (response.body().query().wikitext() == null) {
+ Throwable t = new JsonParseException("Error parsing
wikitext from query response");
cb.failure(call, t);
return;
}
- cb.success(call, response.body().wikitext());
+ cb.success(call, response.body().query().wikitext());
} else {
cb.failure(call, RetrofitException.httpError(response,
cachedService.retrofit()));
}
}
@Override
- public void onFailure(Call<Wikitext> call, Throwable t) {
+ public void onFailure(Call<MwQueryResponse<Wikitext>> call,
Throwable t) {
cb.failure(call, t);
}
});
@@ -55,13 +57,12 @@
}
public interface Callback {
- void success(@NonNull Call<Wikitext> call, @NonNull String wikitext);
- void failure(@NonNull Call<Wikitext> call, @NonNull Throwable caught);
+ void success(@NonNull Call<MwQueryResponse<Wikitext>> call, @NonNull
String wikitext);
+ void failure(@NonNull Call<MwQueryResponse<Wikitext>> call, @NonNull
Throwable caught);
}
private interface Service {
@GET("w/api.php?action=query&format=json&prop=revisions&rvprop=content&rvlimit=1")
- Call<Wikitext> wikitext(@NonNull @Query("titles") String title,
- @Query("rvsection") int section);
+ Call<MwQueryResponse<Wikitext>> request(@NonNull @Query("titles")
String title, @Query("rvsection") int section);
}
}
--
To view, visit https://gerrit.wikimedia.org/r/331014
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1955924c8b3230a536acaa9b44218db7ca3a663b
Gerrit-PatchSet: 3
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mholloway <[email protected]>
Gerrit-Reviewer: BearND <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: Dbrant <[email protected]>
Gerrit-Reviewer: Niedzielski <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
