Mholloway has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/370971 )

Change subject: Remove incorrect RESTBase error handling & preserve error bodies
......................................................................

Remove incorrect RESTBase error handling & preserve error bodies

Several RESTBase response model classes currently expect an error response
object to be contained in an "error" field similar to the MediaWiki API's
error response structure, but RESTBase doesn't return errors this way; it
simply sends the error object.  Hence it will never be picked up in the
way it's currently expected.

This removes the code described above and updates the HttpStatusException
constructor to preserve the error response body (rather than discarding it
as it does now) so that the additional error information from RESTBase is
available if needed.

Bug: T150859
Change-Id: I154fa0df9f7cc4b9dca307b6bf5497993babc50c
---
M app/src/main/java/org/wikipedia/dataclient/mwapi/MwServiceError.java
M app/src/main/java/org/wikipedia/dataclient/okhttp/HttpStatusException.java
M app/src/main/java/org/wikipedia/dataclient/restbase/RbDefinition.java
M app/src/main/java/org/wikipedia/dataclient/restbase/RbServiceError.java
M app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageLead.java
M app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageSummary.java
6 files changed, 32 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia 
refs/changes/71/370971/1

diff --git 
a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwServiceError.java 
b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwServiceError.java
index 4326622..24ce4a0 100644
--- a/app/src/main/java/org/wikipedia/dataclient/mwapi/MwServiceError.java
+++ b/app/src/main/java/org/wikipedia/dataclient/mwapi/MwServiceError.java
@@ -5,6 +5,7 @@
 
 import org.apache.commons.lang3.StringUtils;
 import org.wikipedia.dataclient.ServiceError;
+import org.wikipedia.model.BaseModel;
 
 import java.util.Collections;
 import java.util.List;
@@ -12,7 +13,7 @@
 /**
  * Gson POJO for a MediaWiki API error.
  */
-public class MwServiceError implements ServiceError {
+public class MwServiceError extends BaseModel implements ServiceError {
     @SuppressWarnings("unused") @Nullable private String code;
     @SuppressWarnings("unused") @Nullable private String info;
     @SuppressWarnings("unused") @Nullable private String docref;
@@ -50,14 +51,6 @@
             }
         }
         return null;
-    }
-
-    @Override public String toString() {
-        return "MwServiceError{"
-                + "code='" + code + '\''
-                + ", info='" + info + '\''
-                + ", docref='" + docref + '\''
-                + '}';
     }
 
     private static final class Message {
diff --git 
a/app/src/main/java/org/wikipedia/dataclient/okhttp/HttpStatusException.java 
b/app/src/main/java/org/wikipedia/dataclient/okhttp/HttpStatusException.java
index 36c5ee0..b3e2305 100644
--- a/app/src/main/java/org/wikipedia/dataclient/okhttp/HttpStatusException.java
+++ b/app/src/main/java/org/wikipedia/dataclient/okhttp/HttpStatusException.java
@@ -8,12 +8,24 @@
 
 public class HttpStatusException extends IOException {
     private final int code;
+    @NonNull private String string = "";
 
     public HttpStatusException(@NonNull Response rsp) {
         this.code = rsp.code();
+        try {
+            this.string = rsp.body().string();
+        } catch (NullPointerException|IOException e) {
+            // let it go
+        }
     }
 
     public int code() {
         return code;
     }
+
+    // Can be used to obtain the body of the RESTBase error response, in the 
event this is a
+    // RESTBase error
+    @NonNull public String string() {
+        return string;
+    }
 }
diff --git 
a/app/src/main/java/org/wikipedia/dataclient/restbase/RbDefinition.java 
b/app/src/main/java/org/wikipedia/dataclient/restbase/RbDefinition.java
index cbedbc3..b68a06e 100644
--- a/app/src/main/java/org/wikipedia/dataclient/restbase/RbDefinition.java
+++ b/app/src/main/java/org/wikipedia/dataclient/restbase/RbDefinition.java
@@ -4,13 +4,11 @@
 import android.support.annotation.Nullable;
 
 import org.wikipedia.json.annotations.Required;
-import org.wikipedia.util.log.L;
 
 import java.util.Map;
 
 public class RbDefinition {
     @Required @NonNull private Map<String, Usage[]> usagesByLang;
-    @SuppressWarnings("unused") @Nullable private RbServiceError error;
 
     public RbDefinition(@NonNull Map<String, RbDefinition.Usage[]> usages) {
         usagesByLang = usages;
@@ -18,17 +16,6 @@
 
     @Nullable public Usage[] getUsagesForLang(String langCode) {
         return usagesByLang.get(langCode);
-    }
-
-    public boolean hasError() {
-        return error != null;
-    }
-
-    public void logError(String message) {
-        if (error != null) {
-            message += ": " + error.toString();
-        }
-        L.e(message);
     }
 
     public static class Usage {
diff --git 
a/app/src/main/java/org/wikipedia/dataclient/restbase/RbServiceError.java 
b/app/src/main/java/org/wikipedia/dataclient/restbase/RbServiceError.java
index 1f6b826..8b957aa 100644
--- a/app/src/main/java/org/wikipedia/dataclient/restbase/RbServiceError.java
+++ b/app/src/main/java/org/wikipedia/dataclient/restbase/RbServiceError.java
@@ -1,16 +1,24 @@
 package org.wikipedia.dataclient.restbase;
 
+import android.support.annotation.NonNull;
+
 import org.wikipedia.dataclient.ServiceError;
+import org.wikipedia.json.GsonUnmarshaller;
+import org.wikipedia.model.BaseModel;
 
 /**
  * Gson POJO for a RESTBase API error.
  */
-public class RbServiceError implements ServiceError {
+public class RbServiceError extends BaseModel implements ServiceError {
     @SuppressWarnings("unused") private String type;
     @SuppressWarnings("unused") private String title;
     @SuppressWarnings("unused") private String detail;
     @SuppressWarnings("unused") private String method;
     @SuppressWarnings("unused") private String uri;
+
+    public static RbServiceError create(@NonNull String rspBody) {
+        return GsonUnmarshaller.unmarshal(RbServiceError.class, rspBody);
+    }
 
     @Override
     public String getTitle() {
@@ -20,16 +28,5 @@
     @Override
     public String getDetails() {
         return detail;
-    }
-
-    @Override
-    public String toString() {
-        return "RbServiceError{"
-                + "title='" + title + '\''
-                + ", detail='" + detail + '\''
-                + ", method='" + method + '\''
-                + ", type='" + type + '\''
-                + ", uri='" + uri + '\''
-                + '}';
     }
 }
diff --git 
a/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageLead.java 
b/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageLead.java
index dab0474..256522c 100644
--- a/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageLead.java
+++ b/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageLead.java
@@ -8,11 +8,11 @@
 import com.google.gson.annotations.SerializedName;
 
 import org.wikipedia.auth.AccountUtil;
+import org.wikipedia.dataclient.ServiceError;
 import org.wikipedia.dataclient.WikiSite;
 import org.wikipedia.dataclient.page.PageLead;
 import org.wikipedia.dataclient.page.PageLeadProperties;
 import org.wikipedia.dataclient.page.Protection;
-import org.wikipedia.dataclient.restbase.RbServiceError;
 import org.wikipedia.page.GeoTypeAdapter;
 import org.wikipedia.page.Namespace;
 import org.wikipedia.page.Page;
@@ -20,7 +20,6 @@
 import org.wikipedia.page.PageTitle;
 import org.wikipedia.page.Section;
 import org.wikipedia.util.UriUtil;
-import org.wikipedia.util.log.L;
 
 import java.util.Collections;
 import java.util.List;
@@ -32,7 +31,6 @@
  * Gson POJO for loading the first stage of page content.
  */
 public class RbPageLead implements PageLead, PageLeadProperties {
-    @SuppressWarnings("unused") private RbServiceError error;
     @SuppressWarnings("unused") private int id;
     @SuppressWarnings("unused") private long revision;
     @SuppressWarnings("unused") @Nullable private String lastmodified;
@@ -53,21 +51,18 @@
 
     @Override
     public boolean hasError() {
-        return error != null || sections == null;
+        // If we have a page lead object, RESTBase hasn't returned an error
+        return false;
     }
 
     @Override
-    @Nullable
-    public RbServiceError getError() {
-        return error;
+    public ServiceError getError() {
+        return null;
     }
 
     @Override
     public void logError(String message) {
-        if (error != null) {
-            message += ": " + error.toString();
-        }
-        L.e(message);
+
     }
 
     /** Note: before using this check that #getMobileview != null */
diff --git 
a/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageSummary.java 
b/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageSummary.java
index d19b5b4..dcb3894 100644
--- 
a/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageSummary.java
+++ 
b/app/src/main/java/org/wikipedia/dataclient/restbase/page/RbPageSummary.java
@@ -16,7 +16,6 @@
  * is sent is "normalizedtitle".
  */
 public class RbPageSummary implements PageSummary {
-    @SuppressWarnings("unused") private RbServiceError error;
     @SuppressWarnings("unused,NullableProblems") @Required @NonNull private 
String title;
     @SuppressWarnings("unused") @Nullable private String extract;
     @SuppressWarnings("unused") @Nullable private String description;
@@ -24,13 +23,13 @@
 
     @Override
     public boolean hasError() {
-        // if there is no title or no extract set something went terribly wrong
-        return error != null || extract == null;
+        // If we have a page summary object, RESTBase hasn't returned an error
+        return false;
     }
 
     @Override @Nullable
     public RbServiceError getError() {
-        return error;
+        return null;
     }
 
     @Override @NonNull

-- 
To view, visit https://gerrit.wikimedia.org/r/370971
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I154fa0df9f7cc4b9dca307b6bf5497993babc50c
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

Reply via email to