Bug 1244944 - Don't throw ParseException from ExtendedJSONObject. r?rnewman draft
authorNick Alexander <nalexander@mozilla.com>
Wed, 20 Jan 2016 17:02:55 -0800
changeset 327804 1c362221f096ab0da8c338fae1dff2cf8e043fee
parent 327803 9514d4c4a8fcdad570e30b5ff9c45e7721adcfa4
child 327805 34d94568c1c6ee6c2b571a56a5a36b371a9115c2
push id10307
push usernalexander@mozilla.com
push dateTue, 02 Feb 2016 01:08:05 +0000
reviewersrnewman
bugs1244944
milestone47.0a1
Bug 1244944 - Don't throw ParseException from ExtendedJSONObject. r?rnewman One step further on the path of removing dependence on org.json.simple: don't expose its exceptions to consumers of EJO. We re-purpose the existing UnexpectedJSONException classes to incorporate what were once ParseException instances.
mobile/android/services/src/main/java/org/mozilla/gecko/sync/ExtendedJSONObject.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/NonArrayJSONException.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/NonObjectJSONException.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/UnexpectedJSONException.java
mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/RepoUtils.java
mobile/android/services/src/main/java/org/mozilla/gecko/tokenserver/TokenServerClient.java
mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestMetaGlobal.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/stage/test/TestFetchMetaGlobalStage.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/test/TestExtendedJSONObject.java
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/ExtendedJSONObject.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/ExtendedJSONObject.java
@@ -101,71 +101,80 @@ public class ExtendedJSONObject {
   }
 
   /**
    * Helper method to get a JSON array from a string.
    * <p>
    * You should prefer the stream interface {@link #parseJSONArray(Reader)}.
    *
    * @param jsonString input.
-   * @throws ParseException
    * @throws IOException
-   * @throws NonArrayJSONException if the object is valid JSON, but not an array.
+   * @throws NonArrayJSONException if the object is invalid JSON or not an array.
    */
   public static JSONArray parseJSONArray(String jsonString)
-      throws IOException, ParseException, NonArrayJSONException {
-    Object o = parseRaw(jsonString);
+      throws IOException, NonArrayJSONException {
+    Object o = null;
+    try {
+      o = parseRaw(jsonString);
+    } catch (ParseException e) {
+      throw new NonArrayJSONException(e);
+    }
 
     if (o == null) {
       return null;
     }
 
     if (o instanceof JSONArray) {
       return (JSONArray) o;
     }
 
     throw new NonArrayJSONException("value must be a JSON array");
   }
 
   /**
    * Helper method to get a JSON object from a UTF-8 byte array.
    *
    * @param in UTF-8 bytes.
-   * @throws ParseException
-   * @throws NonObjectJSONException if the object is valid JSON, but not an object.
+   * @throws NonObjectJSONException if the object is not valid JSON or not an object.
    * @throws IOException
    */
   public static ExtendedJSONObject parseUTF8AsJSONObject(byte[] in)
-      throws ParseException, NonObjectJSONException, IOException {
+      throws NonObjectJSONException, IOException {
     return new ExtendedJSONObject(new String(in, "UTF-8"));
   }
 
   public ExtendedJSONObject() {
     this.object = new JSONObject();
   }
 
   public ExtendedJSONObject(JSONObject o) {
     this.object = o;
   }
 
-  public ExtendedJSONObject(Reader in) throws IOException, ParseException, NonObjectJSONException {
+  public ExtendedJSONObject(Reader in) throws IOException, NonObjectJSONException {
     if (in == null) {
       this.object = new JSONObject();
       return;
     }
 
-    Object obj = parseRaw(in);
+    Object obj = null;
+    try {
+      obj = parseRaw(in);
+    } catch (ParseException e) {
+      throw new NonObjectJSONException(e);
+    }
+
     if (obj instanceof JSONObject) {
       this.object = ((JSONObject) obj);
     } else {
       throw new NonObjectJSONException("value must be a JSON object");
     }
   }
 
-  public ExtendedJSONObject(String jsonString) throws IOException, ParseException, NonObjectJSONException {
+  public ExtendedJSONObject(String jsonString) throws IOException, NonObjectJSONException {
     this(jsonString == null ? null : new StringReader(jsonString));
   }
 
   @Override
   public ExtendedJSONObject clone() {
     return new ExtendedJSONObject((JSONObject) this.object.clone());
   }
 
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/NonArrayJSONException.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/NonArrayJSONException.java
@@ -5,9 +5,13 @@
 package org.mozilla.gecko.sync;
 
 public class NonArrayJSONException extends UnexpectedJSONException {
   private static final long serialVersionUID = 5582918057432365749L;
 
   public NonArrayJSONException(String detailMessage) {
     super(detailMessage);
   }
+
+  public NonArrayJSONException(Throwable throwable) {
+    super(throwable);
+  }
 }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/NonObjectJSONException.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/NonObjectJSONException.java
@@ -5,9 +5,13 @@
 package org.mozilla.gecko.sync;
 
 public class NonObjectJSONException extends UnexpectedJSONException {
   private static final long serialVersionUID = 2214238763035650087L;
 
   public NonObjectJSONException(String detailMessage) {
     super(detailMessage);
   }
+
+  public NonObjectJSONException(Throwable throwable) {
+    super(throwable);
+  }
 }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/UnexpectedJSONException.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/UnexpectedJSONException.java
@@ -6,16 +6,20 @@ package org.mozilla.gecko.sync;
 
 public class UnexpectedJSONException extends Exception {
   private static final long serialVersionUID = 4797570033096443169L;
 
   public UnexpectedJSONException(String detailMessage) {
     super(detailMessage);
   }
 
+  public UnexpectedJSONException(Throwable throwable) {
+    super(throwable);
+  }
+
   public static class BadRequiredFieldJSONException extends UnexpectedJSONException {
     private static final long serialVersionUID = -9207736984784497612L;
 
     public BadRequiredFieldJSONException(String string) {
       super(string);
     }
   }
 }
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/RepoUtils.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/repositories/android/RepoUtils.java
@@ -134,19 +134,16 @@ public class RepoUtils {
     try {
       return ExtendedJSONObject.parseJSONArray(getStringFromCursor(cur, colId));
     } catch (NonArrayJSONException e) {
       Logger.error(LOG_TAG, "JSON parsing error for " + colId, e);
       return null;
     } catch (IOException e) {
       Logger.error(LOG_TAG, "JSON parsing error for " + colId, e);
       return null;
-    } catch (ParseException e) {
-      Logger.error(LOG_TAG, "JSON parsing error for " + colId, e);
-      return null;
     }
   }
 
   /**
    * Return true if the provided URI is non-empty and acceptable to Fennec
    * (i.e., not an undesirable scheme).
    *
    * This code is pilfered from Fennec, which pilfered from Places.
--- a/mobile/android/services/src/main/java/org/mozilla/gecko/tokenserver/TokenServerClient.java
+++ b/mobile/android/services/src/main/java/org/mozilla/gecko/tokenserver/TokenServerClient.java
@@ -166,17 +166,17 @@ public class TokenServerClient {
           for (Object error : result.getArray(JSON_KEY_ERRORS)) {
             Logger.warn(LOG_TAG, "" + error);
 
             if (error instanceof JSONObject) {
               errorList.add(new ExtendedJSONObject((JSONObject) error));
             }
           }
         } catch (NonArrayJSONException e) {
-          Logger.warn(LOG_TAG, "Got non-JSON array '" + result.getString(JSON_KEY_ERRORS) + "'.", e);
+          Logger.warn(LOG_TAG, "Got non-JSON array '" + JSON_KEY_ERRORS + "'.", e);
         }
       }
 
       if (statusCode == 400) {
         throw new TokenServerMalformedRequestException(errorList, result.toJSONString());
       }
 
       if (statusCode == 401) {
--- a/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestMetaGlobal.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/android/sync/net/test/TestMetaGlobal.java
@@ -1,24 +1,24 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 package org.mozilla.android.sync.net.test;
 
-import org.json.simple.parser.ParseException;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.android.sync.test.helpers.HTTPServerTestHelper;
 import org.mozilla.android.sync.test.helpers.MockServer;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
 import org.mozilla.gecko.background.testhelpers.WaitHelper;
 import org.mozilla.gecko.sync.CryptoRecord;
 import org.mozilla.gecko.sync.ExtendedJSONObject;
 import org.mozilla.gecko.sync.MetaGlobal;
+import org.mozilla.gecko.sync.NonObjectJSONException;
 import org.mozilla.gecko.sync.delegates.MetaGlobalDelegate;
 import org.mozilla.gecko.sync.net.BaseResource;
 import org.mozilla.gecko.sync.net.BasicAuthHeaderProvider;
 import org.mozilla.gecko.sync.net.SyncStorageResponse;
 import org.simpleframework.http.Request;
 import org.simpleframework.http.Response;
 
 import java.util.HashSet;
@@ -28,18 +28,16 @@ import java.util.concurrent.atomic.Atomi
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(TestRunner.class)
 public class TestMetaGlobal {
-  public static Object monitor = new Object();
-
   private static final int    TEST_PORT    = HTTPServerTestHelper.getTestPort();
   private static final String TEST_SERVER  = "http://localhost:" + TEST_PORT;
   private static final String TEST_SYNC_ID = "foobar";
 
   public static final String USER_PASS = "c6o7dvmr2c4ud2fyv6woz2u4zi22bcyd:password";
   public static final String META_URL  = TEST_SERVER + "/1.1/c6o7dvmr2c4ud2fyv6woz2u4zi22bcyd/storage/meta/global";
   private HTTPServerTestHelper data    = new HTTPServerTestHelper();
 
@@ -218,17 +216,17 @@ public class TestMetaGlobal {
     MockServer existingMetaGlobalServer = new MockServer(200, TEST_META_GLOBAL_MALFORMED_PAYLOAD_RESPONSE);
 
     data.startHTTPServer(existingMetaGlobalServer);
     final MockMetaGlobalFetchDelegate delegate = doFetch(global);
     data.stopHTTPServer();
 
     assertTrue(delegate.errorCalled);
     assertNotNull(delegate.errorException);
-    assertEquals(ParseException.class, delegate.errorException.getClass());
+    assertEquals(NonObjectJSONException.class, delegate.errorException.getClass());
   }
 
   @SuppressWarnings("static-method")
   @Test
   public void testSetFromRecord() throws Exception {
     MetaGlobal mg = new MetaGlobal(null, null);
     mg.setFromRecord(CryptoRecord.fromJSONRecord(TEST_META_GLOBAL_RESPONSE));
     assertEquals("zPSQTm7WBVWB", mg.getSyncID());
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/stage/test/TestFetchMetaGlobalStage.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/stage/test/TestFetchMetaGlobalStage.java
@@ -330,17 +330,17 @@ public class TestFetchMetaGlobalStage {
    * @throws Exception
    */
   @Test
   public void testFetchMalformedPayload() throws Exception {
     MockServer server = new MockServer(200, TestMetaGlobal.TEST_META_GLOBAL_MALFORMED_PAYLOAD_RESPONSE);
     doSession(server);
 
     assertEquals(true, callback.calledError);
-    assertEquals(ParseException.class, callback.calledErrorException.getClass());
+    assertEquals(NonObjectJSONException.class, callback.calledErrorException.getClass());
   }
 
   protected void doFreshStart(MockServer server) {
     data.startHTTPServer(server);
     WaitHelper.getTestWaiter().performWait(WaitHelper.onThreadRunnable(new Runnable() {
       @Override
       public void run() {
         session.freshStart();
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/test/TestExtendedJSONObject.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/sync/test/TestExtendedJSONObject.java
@@ -82,17 +82,17 @@ public class TestExtendedJSONObject {
     assertThat((Long) ((JSONObject) result.get(2)).get("test"), is(equalTo(2L)));
   }
 
   @Test
   public void testBadParseJSONArray() throws Exception {
     try {
       ExtendedJSONObject.parseJSONArray("[0, ");
       fail();
-    } catch (ParseException e) {
+    } catch (NonArrayJSONException e) {
       // Do nothing.
     }
 
     try {
       ExtendedJSONObject.parseJSONArray("{}");
       fail();
     } catch (NonArrayJSONException e) {
       // Do nothing.
@@ -108,24 +108,24 @@ public class TestExtendedJSONObject {
     assertEquals("value", o.getString("key"));
   }
 
   @Test
   public void testBadParseUTF8AsJSONObject() throws Exception {
     try {
       ExtendedJSONObject.parseUTF8AsJSONObject("{}".getBytes("UTF-16"));
       fail();
-    } catch (ParseException e) {
+    } catch (NonObjectJSONException e) {
       // Do nothing.
     }
 
     try {
       ExtendedJSONObject.parseUTF8AsJSONObject("{".getBytes("UTF-8"));
       fail();
-    } catch (ParseException e) {
+    } catch (NonObjectJSONException e) {
       // Do nothing.
     }
   }
 
   @Test
   public void testHashCode() throws Exception {
     ExtendedJSONObject o = new ExtendedJSONObject(exampleJSON);
     assertEquals(o.hashCode(), o.hashCode());