Mholloway has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/333380 )
Change subject: Hygiene: Clean up some editing-related classes
......................................................................
Hygiene: Clean up some editing-related classes
* Reorganizes the result handling logic in EditClient for better
readability
* Brings EditAbuseFilterResult up to present-day standards and updates
the constructor to remove the need to remarshal the response to pass in
* Moves the two-second delay out of the data client before calling
cb.success, and out into the caller. Besides being cleaner in general
this has the added benefit of removing unnecessary delays when dealing
with results other than edit success (abuse filter, spam blacklist,
captcha).
Change-Id: I31e5225d5d7280fa5dd0bd67c4231e16450b8918
---
M app/src/main/java/org/wikipedia/edit/Edit.java
M app/src/main/java/org/wikipedia/edit/EditAbuseFilterResult.java
M app/src/main/java/org/wikipedia/edit/EditClient.java
M app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
4 files changed, 104 insertions(+), 77 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia
refs/changes/80/333380/1
diff --git a/app/src/main/java/org/wikipedia/edit/Edit.java
b/app/src/main/java/org/wikipedia/edit/Edit.java
index f6079de..7cbff62 100644
--- a/app/src/main/java/org/wikipedia/edit/Edit.java
+++ b/app/src/main/java/org/wikipedia/edit/Edit.java
@@ -20,6 +20,8 @@
@SuppressWarnings("unused") private int newrevid;
@SuppressWarnings("unused") @Nullable private Captcha captcha;
@SuppressWarnings("unused") @Nullable private String code;
+ @SuppressWarnings("unused") @Nullable private String info;
+ @SuppressWarnings("unused") @Nullable private String warning;
@SuppressWarnings("unused") @Nullable private String spamblacklist;
@Nullable String status() {
@@ -34,7 +36,7 @@
return captcha == null ? null : captcha.id();
}
- boolean hasErrorCode() {
+ boolean hasEditErrorCode() {
return code != null;
}
@@ -42,6 +44,18 @@
return captcha != null;
}
+ @Nullable String code() {
+ return code;
+ }
+
+ @Nullable String info() {
+ return info;
+ }
+
+ @Nullable String warning() {
+ return warning;
+ }
+
@Nullable String spamblacklist() {
return spamblacklist;
}
diff --git a/app/src/main/java/org/wikipedia/edit/EditAbuseFilterResult.java
b/app/src/main/java/org/wikipedia/edit/EditAbuseFilterResult.java
index e3cb395..843c795 100644
--- a/app/src/main/java/org/wikipedia/edit/EditAbuseFilterResult.java
+++ b/app/src/main/java/org/wikipedia/edit/EditAbuseFilterResult.java
@@ -2,45 +2,43 @@
import android.os.Parcel;
import android.os.Parcelable;
+import android.support.annotation.Nullable;
-import org.json.JSONObject;
+class EditAbuseFilterResult extends EditResult {
+ static final int TYPE_WARNING = 1;
+ static final int TYPE_ERROR = 2;
-public class EditAbuseFilterResult extends EditResult {
- public static final int TYPE_WARNING = 1;
- public static final int TYPE_ERROR = 2;
+ @Nullable private final String code;
+ @Nullable private final String info;
+ @Nullable private final String warning;
- private final String code;
- private final String info;
- private final String warning;
-
- public EditAbuseFilterResult(JSONObject result) {
+ EditAbuseFilterResult(@Nullable String code, @Nullable String info,
@Nullable String warning) {
super("Failure");
- this.code = result.optString("code");
- this.info = result.optString("info");
- this.warning = result.optString("warning");
+ this.code = code;
+ this.info = info;
+ this.warning = warning;
}
- protected EditAbuseFilterResult(Parcel in) {
+ private EditAbuseFilterResult(Parcel in) {
super(in);
code = in.readString();
info = in.readString();
warning = in.readString();
}
- public String getCode() {
+ @Nullable public String getCode() {
return code;
}
- public String getInfo() {
+ @Nullable public String getInfo() {
return info;
}
- public String getWarning() {
+ @Nullable public String getWarning() {
return warning;
}
- @Override
- public void writeToParcel(Parcel dest, int flags) {
+ @Override public void writeToParcel(Parcel dest, int flags) {
super.writeToParcel(dest, flags);
dest.writeString(code);
dest.writeString(info);
@@ -48,11 +46,11 @@
}
public int getType() {
- if (code.startsWith("abusefilter-warning")) {
+ if (code != null && code.startsWith("abusefilter-warning")) {
return TYPE_WARNING;
- } else if (code.startsWith("abusefilter-disallowed")) {
+ } else if (code != null && code.startsWith("abusefilter-disallowed")) {
return TYPE_ERROR;
- } else if (info.startsWith("Hit AbuseFilter")) {
+ } else if (info != null && info.startsWith("Hit AbuseFilter")) {
// This case is here because, unfortunately, an admin can create
an abuse filter which
// emits an arbitrary error code over the API.
// TODO: More properly handle the case where the AbuseFilter
throws an arbitrary error.
diff --git a/app/src/main/java/org/wikipedia/edit/EditClient.java
b/app/src/main/java/org/wikipedia/edit/EditClient.java
index e2343eb..bad2fe4 100644
--- a/app/src/main/java/org/wikipedia/edit/EditClient.java
+++ b/app/src/main/java/org/wikipedia/edit/EditClient.java
@@ -4,15 +4,14 @@
import android.support.annotation.Nullable;
import android.support.annotation.VisibleForTesting;
-import org.json.JSONException;
-import org.json.JSONObject;
import org.wikipedia.captcha.CaptchaResult;
import org.wikipedia.dataclient.WikiSite;
+import org.wikipedia.dataclient.mwapi.MwApiException;
import org.wikipedia.dataclient.retrofit.MwCachedService;
-import org.wikipedia.json.GsonMarshaller;
+import org.wikipedia.dataclient.retrofit.RetrofitException;
import org.wikipedia.page.PageTitle;
-import java.util.concurrent.TimeUnit;
+import java.io.IOException;
import retrofit2.Call;
import retrofit2.Response;
@@ -21,59 +20,45 @@
import retrofit2.http.POST;
class EditClient {
- @NonNull private final MwCachedService<Service> cachedService
- = new MwCachedService<>(Service.class);
+ @NonNull private final MwCachedService<Service> cachedService = new
MwCachedService<>(Service.class);
- @SuppressWarnings("checkstyle:parameternumber")
- public Call<Edit> request(@NonNull WikiSite wiki, @NonNull PageTitle
title, int section,
- @NonNull String text, @NonNull String token, @NonNull
String summary,
- boolean loggedIn, @Nullable String captchaId,
@Nullable String captchaWord,
- @NonNull Callback cb) {
- Service service = cachedService.service(wiki);
- return request(service, title, section, text, token, summary,
loggedIn, captchaId, captchaWord, cb);
+ public interface Callback {
+ void success(@NonNull Call<Edit> call, @NonNull EditResult result);
+ void failure(@NonNull Call<Edit> call, @NonNull Throwable caught);
}
@SuppressWarnings("checkstyle:parameternumber")
- @VisibleForTesting Call<Edit> request(@NonNull Service service, @NonNull
PageTitle title, int section,
+ public Call<Edit> request(@NonNull WikiSite wiki, @NonNull PageTitle
title, int section,
+ @NonNull String text, @NonNull String token,
@NonNull String summary,
+ boolean loggedIn, @Nullable String captchaId,
@Nullable String captchaWord,
+ @NonNull Callback cb) {
+ return request(cachedService.service(wiki), title, section, text,
token, summary, loggedIn,
+ captchaId, captchaWord, cb);
+ }
+
+ @VisibleForTesting @SuppressWarnings("checkstyle:parameternumber")
+ Call<Edit> request(@NonNull Service service, @NonNull PageTitle title, int
section,
@NonNull String text, @NonNull String token, @NonNull
String summary,
boolean loggedIn, @Nullable String captchaId, @Nullable
String captchaWord,
@NonNull final Callback cb) {
- Call<Edit> call = service.edit(title.getPrefixedText(), section, text,
- token, summary, loggedIn ? "user" : null, captchaId,
captchaWord);
+ Call<Edit> call = service.edit(title.getPrefixedText(), section, text,
token, summary,
+ loggedIn ? "user" : null, captchaId, captchaWord);
call.enqueue(new retrofit2.Callback<Edit>() {
@Override
public void onResponse(Call<Edit> call, Response<Edit> response) {
- if (response.isSuccessful() &&
response.body().hasEditResult()) {
- Edit.Result result = response.body().edit();
- if ("Success".equals(result.status())) {
- try {
- // TODO: get edit revision and request that
revision
- Thread.sleep(TimeUnit.SECONDS.toMillis(2));
- cb.success(call, new
EditSuccessResult(result.newRevId()));
- } catch (InterruptedException e) {
- cb.failure(call, e);
- }
- } else if (result.hasErrorCode()) {
- try {
- JSONObject json = new
JSONObject(GsonMarshaller.marshal(result));
- cb.success(call, new EditAbuseFilterResult(json));
- } catch (JSONException e) {
- cb.failure(call, e);
- }
- } else if (result.hasSpamBlacklistResponse()) {
- cb.success(call, new
EditSpamBlacklistResult(result.spamblacklist()));
- } else if (result.hasCaptchaResponse()) {
- cb.success(call, new
CaptchaResult(result.captchaId()));
+ if (response.isSuccessful()) {
+ if (response.body().hasEditResult()) {
+ handleEditResult(response.body().edit(), call, cb);
+ } else if (response.body().hasError()) {
+ RuntimeException e = isLoginFailure(response)
+ ? new UserNotLoggedInException()
+ : new
MwApiException(response.body().getError());
+ cb.failure(call, e);
} else {
- cb.failure(call, new RuntimeException("Received
unrecognized edit response"));
+ cb.failure(call, new IOException("An unknown error
occurred."));
}
- } else if ("assertuserfailed".equals(response.body().code())) {
- cb.failure(call, new UserNotLoggedInException());
- } else if (response.body().info() != null) {
- String info = response.body().info();
- cb.failure(call, new RuntimeException(info));
} else {
- cb.failure(call, new RuntimeException("Received
unrecognized edit response"));
+ cb.failure(call, RetrofitException.httpError(response,
cachedService.retrofit()));
}
}
@@ -85,9 +70,28 @@
return call;
}
- @VisibleForTesting interface Callback {
- void success(@NonNull Call<Edit> call, @NonNull EditResult result);
- void failure(@NonNull Call<Edit> call, @NonNull Throwable caught);
+ private void handleEditResult(@NonNull Edit.Result result, @NonNull
Call<Edit> call,
+ @NonNull Callback cb) {
+ if (isEditSuccessResult(result)) {
+ // TODO: get edit revision and request that revision
+ cb.success(call, new EditSuccessResult(result.newRevId()));
+ } else if (result.hasEditErrorCode()) {
+ cb.success(call, new EditAbuseFilterResult(result.code(),
result.info(), result.warning()));
+ } else if (result.hasSpamBlacklistResponse()) {
+ cb.success(call, new
EditSpamBlacklistResult(result.spamblacklist()));
+ } else if (result.hasCaptchaResponse()) {
+ cb.success(call, new CaptchaResult(result.captchaId()));
+ } else {
+ cb.failure(call, new IOException("Received unrecognized edit
response"));
+ }
+ }
+
+ private boolean isEditSuccessResult(@NonNull Edit.Result result) {
+ return "Success".equals(result.status());
+ }
+
+ private boolean isLoginFailure(@NonNull Response<Edit> response) {
+ return "assertuserfailed".equals(response.body().code());
}
@VisibleForTesting interface Service {
diff --git a/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
b/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
index 1f3d767..40101e6 100644
--- a/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
+++ b/app/src/main/java/org/wikipedia/edit/EditSectionActivity.java
@@ -6,6 +6,7 @@
import android.graphics.Typeface;
import android.net.Uri;
import android.os.Bundle;
+import android.os.Handler;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.content.ContextCompat;
@@ -54,6 +55,8 @@
import org.wikipedia.util.StringUtil;
import org.wikipedia.util.log.L;
import org.wikipedia.views.ViewAnimations;
+
+import java.util.concurrent.TimeUnit;
import retrofit2.Call;
@@ -108,6 +111,19 @@
private EditFunnel funnel;
private ProgressDialog progressDialog;
+
+ private Runnable successRunnable = new Runnable() {
+ @Override public void run() {
+ progressDialog.dismiss();
+
+ //Build intent that includes the section we were editing, so we
can scroll to it later
+ Intent data = new Intent();
+ data.putExtra(EXTRA_SECTION_ID, sectionID);
+ setResult(EditHandler.RESULT_REFRESH_PAGE, data);
+ hideSoftKeyboard(EditSectionActivity.this);
+ finish();
+ }
+ };
@Override
public void onCreate(Bundle savedInstanceState) {
@@ -300,14 +316,9 @@
}
if (result instanceof EditSuccessResult) {
funnel.logSaved(((EditSuccessResult)
result).getRevID());
- progressDialog.dismiss();
-
- //Build intent that includes the section
we were editing, so we can scroll to it later
- Intent data = new Intent();
- data.putExtra(EXTRA_SECTION_ID, sectionID);
- setResult(EditHandler.RESULT_REFRESH_PAGE,
data);
- hideSoftKeyboard(EditSectionActivity.this);
- finish();
+ // TODO: remove this artificial delay and
use the new revision
+ // ID returned to request the updated
version of the page
+ new Handler().postDelayed(successRunnable,
TimeUnit.SECONDS.toMillis(2));
} else if (result instanceof CaptchaResult) {
if (captchaHandler.isActive()) {
// Captcha entry failed!
--
To view, visit https://gerrit.wikimedia.org/r/333380
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I31e5225d5d7280fa5dd0bd67c4231e16450b8918
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mholloway <mhollo...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
