Brion VIBBER has submitted this change and it was merged.
Change subject: Create wrapper exception to wrap around JSONException
......................................................................
Create wrapper exception to wrap around JSONException
Since most of the time we will be handling the various exceptions
that can pop up in the same way ('operation failed, please
try again'), and the actual errors returned by MediaWiki will
be in the JSON structure itself and need to be handled separately,
it makes sense to have a single wrapper exception to reduce the
number of catch blocks absolutely needed. Most client code
can probably treat the two wrapped exceptions the same and not
lose much.
Wrapping the other exception (IOException) will be done in next
commit.
Change-Id: Id74be171dffd670277e0b49a492e3813ceed983a
---
M src/main/java/org/mediawiki/api/Api.java
A src/main/java/org/mediawiki/api/ApiException.java
M src/main/java/org/mediawiki/api/RequestBuilder.java
M src/test/java/org/mediawiki/api/ApiTest.java
4 files changed, 65 insertions(+), 9 deletions(-)
Approvals:
Brion VIBBER: Verified; Looks good to me, approved
diff --git a/src/main/java/org/mediawiki/api/Api.java
b/src/main/java/org/mediawiki/api/Api.java
index a39537e..918de1f 100644
--- a/src/main/java/org/mediawiki/api/Api.java
+++ b/src/main/java/org/mediawiki/api/Api.java
@@ -105,9 +105,10 @@
*
* @param method HTTP method to use when performing the request
* @param requestBuilder The requestBuilder to use to construct the request
+ * @throws ApiException In case of NetworkError or JSON parsing error
* @return A JSONObject with the response from the server
*/
- public JSONObject makeRequest(final int method, final RequestBuilder
requestBuilder) {
+ public JSONObject makeRequest(final int method, final RequestBuilder
requestBuilder) throws ApiException {
HttpRequest request;
switch(method) {
case METHOD_GET:
@@ -122,8 +123,7 @@
try {
return new JSONObject(request.body());
} catch (JSONException e) {
- // Well, if the request *does* complete successfully but the
- throw new RuntimeException(e);
+ throw new ApiException(e);
}
}
}
diff --git a/src/main/java/org/mediawiki/api/ApiException.java
b/src/main/java/org/mediawiki/api/ApiException.java
new file mode 100644
index 0000000..0f4944c
--- /dev/null
+++ b/src/main/java/org/mediawiki/api/ApiException.java
@@ -0,0 +1,32 @@
+package org.mediawiki.api;
+
+import org.json.JSONException;
+
+import java.io.IOException;
+
+/**
+ * Wrapper exception thrown whenever there's an unrecoverable error from the
Api.
+ *
+ * With sadness in my heart, I make this a checked exception, since this does
+ * need to be handled, and it is encapsulating a couple of other checked
+ * exceptions.
+ */
+public class ApiException extends Exception {
+ /**
+ * Create a wrapper around an IOException.
+ *
+ * @param cause IOException around which this is wrapped.
+ */
+ public ApiException(final IOException cause) {
+ super(cause);
+ }
+
+ /**
+ * Create a wrapper around a JSONException.
+ *
+ * @param cause JSONException around which this is wrapped.
+ */
+ public ApiException(final JSONException cause) {
+ super(cause);
+ }
+}
diff --git a/src/main/java/org/mediawiki/api/RequestBuilder.java
b/src/main/java/org/mediawiki/api/RequestBuilder.java
index a338ac2..ab1b817 100644
--- a/src/main/java/org/mediawiki/api/RequestBuilder.java
+++ b/src/main/java/org/mediawiki/api/RequestBuilder.java
@@ -54,27 +54,30 @@
* Performs the request that has been built up so far and returns the JSON
Object.
*
* @param method HTTP Method to use when performing the request
+ * @throws ApiException In case of NetworkError or JSON parsing error
* @return The result of the API request
*/
- private JSONObject makeRequest(final int method) {
+ private JSONObject makeRequest(final int method) throws ApiException {
return api.makeRequest(method, this);
}
/**
* Performs a GET request using the parameters so far specified.
*
+ * @throws ApiException In case of NetworkError or JSON parsing error
* @return The result of the API request
*/
- public JSONObject get() {
+ public JSONObject get() throws ApiException {
return makeRequest(Api.METHOD_GET);
}
/**
* Performs a POST request using the parameters so far specified.
*
+ * @throws ApiException In case of NetworkError or JSON parsing error
* @return The result of the API request
*/
- public JSONObject post() {
+ public JSONObject post() throws ApiException {
return makeRequest(Api.METHOD_POST);
}
}
diff --git a/src/test/java/org/mediawiki/api/ApiTest.java
b/src/test/java/org/mediawiki/api/ApiTest.java
index 1736d68..057fbd2 100644
--- a/src/test/java/org/mediawiki/api/ApiTest.java
+++ b/src/test/java/org/mediawiki/api/ApiTest.java
@@ -2,6 +2,7 @@
import static org.junit.Assert.*;
+import org.json.JSONException;
import org.json.JSONObject;
import org.junit.*;
@@ -11,7 +12,7 @@
public class ApiTest {
@Test
- public void testBasicPost() {
+ public void testBasicPost() throws Exception {
Api api = new Api("test.wikipedia.org");
// We're just checking if the POST goes through, so does not
// matter which username password we use
@@ -27,7 +28,7 @@
}
@Test
- public void testWrongMethod() {
+ public void testWrongMethod() throws Exception {
Api api = new Api("test.wikipedia.org");
JSONObject resp = api.action("login")
.get();
@@ -49,8 +50,28 @@
}
@Test(expected = IllegalArgumentException.class)
- public void testInvalidMethod() {
+ public void testInvalidMethod() throws Exception {
Api api = new Api("test.wikipedia.org");
api.makeRequest(404, null);
}
+
+ /**
+ * This tests for responses that aren't JSON, but something else.
+ *
+ * Usually this happens when a backend apache times out and we get the
+ * timeout HTML page from the frontend caches. Since that is not
+ * reliably reproducible, and we aren't using network mocks, I can
+ * simulate it by simply requesting for the XML format.
+ */
+ @Test
+ public void testJSONException() {
+ Api api = new Api("test.wikipedia.org");
+ try {
+ api.action("somethingdoesnmtatter").param("format", "xml").get();
+ } catch (ApiException e) {
+ assertTrue(e.getCause() instanceof JSONException);
+ return;
+ }
+ assertTrue("This means no exception was thrown, and hence test fails",
false);
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/92477
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id74be171dffd670277e0b49a492e3813ceed983a
Gerrit-PatchSet: 3
Gerrit-Project: apps/android/java-mwapi
Gerrit-Branch: master
Gerrit-Owner: Yuvipanda <[email protected]>
Gerrit-Reviewer: Brion VIBBER <[email protected]>
Gerrit-Reviewer: Yuvipanda <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
