Merge pull request #44 from bugsnag/simplify-json-stream

Simplify JsonStream, avoid unboxing errors, propagate exceptions more sanely, add test
bugsnag · Jan 9, 2015 · 10c42bd · 10c42bd
2 parents 1876f91 + 4c00f8f
commit 10c42bd
Show file tree

Hide file tree

Showing 21 changed files with 165 additions and 141 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -105,7 +105,6 @@ If you are a project maintainer, you can build and release a new version of
 -   "Promote" the release build on Maven Central
 
     -   Go to the [sonatype open source dashboard](https://oss.sonatype.org/index.html#stagingRepositories)
-    -   Click “Staging Repositories”
     -   Click the search box at the top right, and type “com.bugsnag”
     -   Select the com.bugsnag staging repository
     -   Click the “close” button in the toolbar, no message

diff --git a/src/androidTest/java/com/bugsnag/android/BugsnagTestCase.java b/src/androidTest/java/com/bugsnag/android/BugsnagTestCase.java
@@ -1,5 +1,6 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
 import java.io.StringWriter;
 
 import org.json.JSONArray;
@@ -9,10 +10,9 @@
 import android.test.AndroidTestCase;
 
 public class BugsnagTestCase extends AndroidTestCase {
-    protected String streamableToString(JsonStream.Streamable streamable) {
+    protected String streamableToString(JsonStream.Streamable streamable) throws IOException {
         StringWriter writer = new StringWriter();
         JsonStream jsonStream = new JsonStream(writer);
-
         streamable.toStream(jsonStream);
 
         return writer.toString();
@@ -23,7 +23,7 @@ protected JSONObject streamableToJson(JsonStream.Streamable streamable) {
 
         try {
             return new JSONObject(streamableToString(streamable));
-        } catch (JSONException e) {
+        } catch (Exception e) {
             e.printStackTrace();
         }
 
@@ -35,7 +35,7 @@ protected JSONArray streamableToJsonArray(JsonStream.Streamable streamable) {
 
         try {
             return new JSONArray(streamableToString(streamable));
-        } catch (JSONException e) {
+        } catch (Exception e) {
             e.printStackTrace();
         }
 

diff --git a/src/androidTest/java/com/bugsnag/android/JsonStreamTest.java b/src/androidTest/java/com/bugsnag/android/JsonStreamTest.java
@@ -0,0 +1,45 @@
+package com.bugsnag.android;
+
+import java.io.IOException;
+import java.io.StringWriter;
+
+import org.json.JSONException;
+import org.json.JSONObject;
+
+public class JsonStreamTest extends BugsnagTestCase {
+    public void testSaneValues() throws JSONException, IOException {
+        StringWriter writer = new StringWriter();
+        JsonStream stream = new JsonStream(writer);
+
+        Long nullLong = null;
+        Boolean nullBoolean = null;
+        String nullString = null;
+        Integer nullInteger = null;
+        Float nullFloat = null;
+        Double nullDouble = null;
+
+        stream.beginObject();
+        stream.name("nullLong").value(nullLong);
+        stream.name("nullBoolean").value(nullBoolean);
+        stream.name("nullString").value(nullString);
+        stream.name("nullInteger").value(nullInteger);
+        stream.name("nullFloat").value(nullFloat);
+        stream.name("nullDouble").value(nullDouble);
+        stream.name("string").value("string");
+        stream.name("int").value(123);
+        stream.name("long").value(123l);
+        stream.name("float").value(123.45f);
+        stream.endObject();
+
+        JSONObject json = new JSONObject(writer.toString());
+        assertTrue(json.isNull("nullLong"));
+        assertTrue(json.isNull("nullBoolean"));
+        assertTrue(json.isNull("nullString"));
+        assertTrue(json.isNull("nullInteger"));
+        assertTrue(json.isNull("nullFloat"));
+        assertTrue(json.isNull("nullDouble"));
+        assertEquals("string", json.getString("string"));
+        assertEquals(123, json.getInt("int"));
+        assertEquals(123l, json.getLong("long"));
+    }
+}
diff --git a/src/main/java/com/bugsnag/android/AppData.java b/src/main/java/com/bugsnag/android/AppData.java
@@ -1,5 +1,7 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
+
 import android.content.Context;
 import android.content.pm.ApplicationInfo;;
 import android.content.pm.PackageManager;
@@ -32,7 +34,7 @@ class AppData implements JsonStream.Streamable {
         guessedReleaseStage = guessReleaseStage();
     }
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         writer.beginObject();
             writer.name("id").value(packageName);
             writer.name("name").value(appName);

diff --git a/src/main/java/com/bugsnag/android/AppState.java b/src/main/java/com/bugsnag/android/AppState.java
@@ -1,5 +1,6 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
 import java.util.List;
 
 import android.app.ActivityManager;
@@ -36,7 +37,7 @@ static void init() {}
         lowMemory = isLowMemory();
     }
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         writer.beginObject();
             writer.name("duration").value(duration);
             writer.name("inForeground").value(inForeground);

diff --git a/src/main/java/com/bugsnag/android/Breadcrumbs.java b/src/main/java/com/bugsnag/android/Breadcrumbs.java
@@ -1,5 +1,6 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
 import java.util.LinkedList;
 import java.util.Date;
 import java.util.List;
@@ -20,7 +21,7 @@ private static class Breadcrumb {
     private List<Breadcrumb> store = new LinkedList<Breadcrumb>();
     private int maxSize = DEFAULT_MAX_SIZE;
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         writer.beginArray();
 
         for(Breadcrumb breadcrumb : store) {

diff --git a/src/main/java/com/bugsnag/android/DeviceData.java b/src/main/java/com/bugsnag/android/DeviceData.java
@@ -1,6 +1,7 @@
 package com.bugsnag.android;
 
 import java.io.File;
+import java.io.IOException;
 import java.util.Locale;
 
 import android.content.Context;
@@ -38,7 +39,7 @@ class DeviceData implements JsonStream.Streamable {
         id = getAndroidId();
     }
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         writer.beginObject();
             writer.name("manufacturer").value(android.os.Build.MANUFACTURER);
             writer.name("brand").value(android.os.Build.BRAND);

diff --git a/src/main/java/com/bugsnag/android/DeviceState.java b/src/main/java/com/bugsnag/android/DeviceState.java
@@ -1,5 +1,6 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
 import java.util.Date;
 
 import android.content.ContentResolver;
@@ -45,7 +46,7 @@ class DeviceState implements JsonStream.Streamable {
         time = getTime();
     }
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         writer.beginObject();
             writer.name("freeMemory").value(freeMemory);
             writer.name("orientation").value(orientation);

diff --git a/src/main/java/com/bugsnag/android/Error.java b/src/main/java/com/bugsnag/android/Error.java
@@ -1,5 +1,7 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
+
 /**
  * Information and associated diagnostics relating to a handled or unhandled
  * Exception.
@@ -30,7 +32,7 @@ public class Error implements JsonStream.Streamable {
         this.exception = exception;
     }
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         // Merge error metaData into global metadata and apply filters
         MetaData mergedMetaData = MetaData.merge(config.metaData, metaData);
         mergedMetaData.setFilters(config.filters);

diff --git a/src/main/java/com/bugsnag/android/ErrorStore.java b/src/main/java/com/bugsnag/android/ErrorStore.java
@@ -74,13 +74,19 @@ void write(Error error) {
         if(path == null) return;
 
         String filename = String.format("%s%d.json", path, System.currentTimeMillis());
+        Writer out = null;
         try {
-            Writer out = new FileWriter(filename);
-            new JsonStream(out).value(error).close();
+            out = new FileWriter(filename);
+
+            JsonStream stream = new JsonStream(out);
+            stream.value(error);
+            stream.close();
 
             Logger.info(String.format("Saved unsent error to disk (%s) ", filename));
         } catch (Exception e) {
             Logger.warn(String.format("Couldn't save unsent error to disk (%s) ", filename), e);
+        } finally {
+            IOUtils.closeQuietly(out);
         }
     }
 }
diff --git a/src/main/java/com/bugsnag/android/ExceptionChain.java b/src/main/java/com/bugsnag/android/ExceptionChain.java
@@ -1,5 +1,7 @@
 package com.bugsnag.android;
 
+import java.io.IOException;
+
 /**
  * Unwrap and serialize exception information and any "cause" exceptions.
  */
@@ -12,7 +14,7 @@ class ExceptionChain implements JsonStream.Streamable {
         this.exception = exception;
     }
 
-    public void toStream(JsonStream writer) {
+    public void toStream(JsonStream writer) throws IOException {
         writer.beginArray();
 
         Throwable currentEx = exception;

diff --git a/src/main/java/com/bugsnag/android/HttpClient.java b/src/main/java/com/bugsnag/android/HttpClient.java
@@ -38,13 +38,7 @@ static void post(String urlString, JsonStream.Streamable payload) throws Network
                 payload.toStream(stream);
                 stream.close();
             } finally {
-                if (out != null) {
-                    try {
-                        out.close();
-                    } catch (IOException e) {
-                        e.printStackTrace();
-                    }
-                }
+                IOUtils.closeQuietly(out);
             }
 
             // End the request, get the response code
@@ -55,9 +49,7 @@ static void post(String urlString, JsonStream.Streamable payload) throws Network
         } catch (IOException e) {
             throw new NetworkException(urlString, e);
         } finally {
-            if(conn != null) {
-                conn.disconnect();
-            }
+            IOUtils.close(conn);
         }
     }
 }
diff --git a/src/main/java/com/bugsnag/android/IOUtils.java b/src/main/java/com/bugsnag/android/IOUtils.java
@@ -0,0 +1,45 @@
+package com.bugsnag.android;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.Reader;
+import java.io.Writer;
+import java.net.HttpURLConnection;
+import java.net.URLConnection;
+
+class IOUtils {
+    private static final int DEFAULT_BUFFER_SIZE = 1024 * 4;
+    private static final int EOF = -1;
+
+    public static void closeQuietly(final Closeable closeable) {
+        try {
+            if (closeable != null) {
+                closeable.close();
+            }
+        } catch (final IOException ioe) {
+            // ignore
+        }
+    }
+
+    public static void close(final URLConnection conn) {
+        if (conn instanceof HttpURLConnection) {
+            ((HttpURLConnection) conn).disconnect();
+        }
+    }
+
+    public static int copy(final Reader input, final Writer output) throws IOException {
+        char[] buffer = new char[DEFAULT_BUFFER_SIZE];
+        long count = 0;
+        int n = 0;
+        while (EOF != (n = input.read(buffer))) {
+            output.write(buffer, 0, n);
+            count += n;
+        }
+
+        if (count > Integer.MAX_VALUE) {
+            return -1;
+        }
+
+        return (int) count;
+    }
+}