From 960fe22b3cd4d96019b62a142714777592643377 Mon Sep 17 00:00:00 2001 From: Richard Newman Date: Tue, 28 May 2013 12:01:34 -0700 Subject: [PATCH] Bug 875401 - Switch HealthReportGenerator to use org.json.JSONObject/JSONArray. r=nalexander --- .../healthreport/HealthReportGenerator.java | 67 +++++--------- .../healthreport/HealthReportUtils.java | 92 +++++++++++++++++++ 2 files changed, 116 insertions(+), 43 deletions(-) diff --git a/mobile/android/base/background/healthreport/HealthReportGenerator.java b/mobile/android/base/background/healthreport/HealthReportGenerator.java index 029fdbc61a4..0c05e1f8bac 100644 --- a/mobile/android/base/background/healthreport/HealthReportGenerator.java +++ b/mobile/android/base/background/healthreport/HealthReportGenerator.java @@ -4,10 +4,8 @@ package org.mozilla.gecko.background.healthreport; -import java.util.HashMap; - -import org.json.simple.JSONArray; -import org.json.simple.JSONObject; +import org.json.JSONException; +import org.json.JSONObject; import org.mozilla.gecko.background.common.log.Logger; import org.mozilla.gecko.background.healthreport.HealthReportStorage.Field; @@ -32,8 +30,9 @@ public class HealthReportGenerator { /** * @return null if no environment could be computed, or else the resulting document. + * @throws JSONException if there was an error adding environment data to the resulting document. */ - public JSONObject generateDocument(long since, long lastPingTime, String profilePath) { + public JSONObject generateDocument(long since, long lastPingTime, String profilePath) throws JSONException { Logger.info(LOG_TAG, "Generating FHR document from " + since + "; last ping " + lastPingTime + ", for profile " + profilePath); ProfileInformationCache cache = new ProfileInformationCache(profilePath); if (!cache.restoreUnlessInitialized()) { @@ -55,8 +54,9 @@ public class HealthReportGenerator { * * * days is a map from date strings to {hash: {measurement: {_v: version, fields...}}}. + * @throws JSONException if there was an error adding environment data to the resulting document. */ - public JSONObject generateDocument(long since, long lastPingTime, Environment currentEnvironment) { + public JSONObject generateDocument(long since, long lastPingTime, Environment currentEnvironment) throws JSONException { Logger.debug(LOG_TAG, "Current environment hash: " + currentEnvironment.getHash()); // We want to map field IDs to some strings as we go. @@ -64,26 +64,21 @@ public class HealthReportGenerator { JSONObject document = new JSONObject(); - // Defeat "unchecked" warnings with JDK7. See Bug 875088. - @SuppressWarnings("unchecked") - HashMap doc = ((HashMap) document); - if (lastPingTime >= HealthReportConstants.EARLIEST_LAST_PING) { - doc.put("lastPingDate", HealthReportUtils.getDateString(lastPingTime)); + document.put("lastPingDate", HealthReportUtils.getDateString(lastPingTime)); } - doc.put("thisPingDate", HealthReportUtils.getDateString(now())); - doc.put("version", PAYLOAD_VERSION); + document.put("thisPingDate", HealthReportUtils.getDateString(now())); + document.put("version", PAYLOAD_VERSION); - doc.put("environments", getEnvironmentsJSON(currentEnvironment, envs)); - doc.put("data", getDataJSON(currentEnvironment, envs, since)); + document.put("environments", getEnvironmentsJSON(currentEnvironment, envs)); + document.put("data", getDataJSON(currentEnvironment, envs, since)); return document; } - @SuppressWarnings("unchecked") protected JSONObject getDataJSON(Environment currentEnvironment, - SparseArray envs, long since) { + SparseArray envs, long since) throws JSONException { SparseArray fields = storage.getFieldsByID(); JSONObject days = getDaysJSON(currentEnvironment, envs, fields, since); @@ -96,8 +91,7 @@ public class HealthReportGenerator { return data; } - @SuppressWarnings("unchecked") - protected JSONObject getDaysJSON(Environment currentEnvironment, SparseArray envs, SparseArray fields, long since) { + protected JSONObject getDaysJSON(Environment currentEnvironment, SparseArray envs, SparseArray fields, long since) throws JSONException { JSONObject days = new JSONObject(); Cursor cursor = storage.getRawEventsSince(since); try { @@ -140,7 +134,7 @@ public class HealthReportGenerator { } final Field field = fields.get(cField); - JSONObject measurement = (JSONObject) envObject.get(field.measurementName); + JSONObject measurement = envObject.optJSONObject(field.measurementName); if (measurement == null) { // We will never have more than one measurement version within a // single environment -- to do so involves changing the build ID. And @@ -151,15 +145,10 @@ public class HealthReportGenerator { envObject.put(field.measurementName, measurement); } if (field.isDiscreteField()) { - JSONArray discrete = (JSONArray) measurement.get(field.fieldName); - if (discrete == null) { - discrete = new JSONArray(); - measurement.put(field.fieldName, discrete); - } if (field.isStringField()) { - discrete.add(cursor.getString(3)); + HealthReportUtils.append(measurement, field.fieldName, cursor.getString(3)); } else if (field.isIntegerField()) { - discrete.add(cursor.getLong(3)); + HealthReportUtils.append(measurement, field.fieldName, cursor.getLong(3)); } else { // Uh oh! throw new IllegalStateException("Unknown field type: " + field.flags); @@ -182,9 +171,8 @@ public class HealthReportGenerator { return days; } - @SuppressWarnings("unchecked") protected JSONObject getEnvironmentsJSON(Environment currentEnvironment, - SparseArray envs) { + SparseArray envs) throws JSONException { JSONObject environments = new JSONObject(); // Always do this, even if it hasn't recorded anything in the DB. @@ -201,8 +189,7 @@ public class HealthReportGenerator { return environments; } - @SuppressWarnings("unchecked") - private JSONObject jsonify(Environment e, Environment current) { + private JSONObject jsonify(Environment e, Environment current) throws JSONException { JSONObject age = getProfileAge(e, current); JSONObject sysinfo = getSysInfo(e, current); JSONObject gecko = getGeckoInfo(e, current); @@ -231,8 +218,7 @@ public class HealthReportGenerator { return out; } - @SuppressWarnings("unchecked") - private JSONObject getProfileAge(Environment e, Environment current) { + private JSONObject getProfileAge(Environment e, Environment current) throws JSONException { JSONObject age = new JSONObject(); int changes = 0; if (current == null || current.profileCreation != e.profileCreation) { @@ -246,8 +232,7 @@ public class HealthReportGenerator { return age; } - @SuppressWarnings("unchecked") - private JSONObject getSysInfo(Environment e, Environment current) { + private JSONObject getSysInfo(Environment e, Environment current) throws JSONException { JSONObject sysinfo = new JSONObject(); int changes = 0; if (current == null || current.cpuCount != e.cpuCount) { @@ -277,8 +262,7 @@ public class HealthReportGenerator { return sysinfo; } - @SuppressWarnings("unchecked") - private JSONObject getGeckoInfo(Environment e, Environment current) { + private JSONObject getGeckoInfo(Environment e, Environment current) throws JSONException { JSONObject gecko = new JSONObject(); int changes = 0; if (current == null || !current.vendor.equals(e.vendor)) { @@ -328,8 +312,7 @@ public class HealthReportGenerator { return gecko; } - @SuppressWarnings("unchecked") - private JSONObject getAppInfo(Environment e, Environment current) { + private JSONObject getAppInfo(Environment e, Environment current) throws JSONException { JSONObject appinfo = new JSONObject(); int changes = 0; if (current == null || current.isBlocklistEnabled != e.isBlocklistEnabled) { @@ -347,8 +330,7 @@ public class HealthReportGenerator { return appinfo; } - @SuppressWarnings("unchecked") - private JSONObject getAddonCounts(Environment e, Environment current) { + private JSONObject getAddonCounts(Environment e, Environment current) throws JSONException { JSONObject counts = new JSONObject(); int changes = 0; if (current == null || current.extensionCount != e.extensionCount) { @@ -370,8 +352,7 @@ public class HealthReportGenerator { return counts; } - @SuppressWarnings("unchecked") - private JSONObject getActiveAddons(Environment e, Environment current) { + private JSONObject getActiveAddons(Environment e, Environment current) throws JSONException { JSONObject active = new JSONObject(); int changes = 0; if (current != null && changes == 0) { diff --git a/mobile/android/base/background/healthreport/HealthReportUtils.java b/mobile/android/base/background/healthreport/HealthReportUtils.java index d61761eee61..1510af77b5b 100644 --- a/mobile/android/base/background/healthreport/HealthReportUtils.java +++ b/mobile/android/base/background/healthreport/HealthReportUtils.java @@ -5,9 +5,17 @@ package org.mozilla.gecko.background.healthreport; import java.text.SimpleDateFormat; +import java.util.HashSet; +import java.util.Iterator; import java.util.Locale; +import java.util.Set; +import java.util.SortedSet; import java.util.TimeZone; +import java.util.TreeSet; +import org.json.JSONException; +import org.json.JSONObject; +import org.json.JSONArray; import org.mozilla.apache.commons.codec.digest.DigestUtils; import android.content.ContentUris; @@ -45,4 +53,88 @@ public class HealthReportUtils { public static Uri getEventURI(Uri environmentURI) { return environmentURI.buildUpon().path("/events/" + ContentUris.parseId(environmentURI) + "/").build(); } + + /** + * Copy the keys from the provided {@link JSONObject} into the provided {@link Set}. + */ + private static > T intoKeySet(T keys, JSONObject o) { + if (o == null || o == JSONObject.NULL) { + return keys; + } + + @SuppressWarnings("unchecked") + Iterator it = o.keys(); + while (it.hasNext()) { + keys.add(it.next()); + } + return keys; + } + + /** + * Produce a {@link SortedSet} containing the string keys of the provided + * object. + * + * @param o a {@link JSONObject} with string keys. + * @return a sorted set. + */ + public static SortedSet sortedKeySet(JSONObject o) { + return intoKeySet(new TreeSet(), o); + } + + /** + * Produce a {@link Set} containing the string keys of the provided object. + * @param o a {@link JSONObject} with string keys. + * @return an unsorted set. + */ + public static Set keySet(JSONObject o) { + return intoKeySet(new HashSet(), o); + } + + /** + * {@link JSONObject} doesn't provide a clone method, nor any + * useful constructors, so we do this the hard way. + * + * @return a new object containing the same keys and values as the old. + * @throws JSONException + * if JSONObject is even more stupid than expected and cannot store + * a value from the provided object in the new object. This should + * never happen. + */ + public static JSONObject shallowCopyObject(JSONObject o) throws JSONException { + if (o == null) { + return null; + } + + JSONObject out = new JSONObject(); + @SuppressWarnings("unchecked") + Iterator keys = out.keys(); + while (keys.hasNext()) { + final String key = keys.next(); + out.put(key, o.get(key)); + } + return out; + } + + /** + * Just like {@link JSONObject#accumulate(String, Object)}, but doesn't do the wrong thing for single values. + * @throws JSONException + */ + public static void append(JSONObject o, String key, Object value) throws JSONException { + if (!o.has(key)) { + JSONArray arr = new JSONArray(); + arr.put(value); + o.put(key, arr); + return; + } + Object dest = o.get(key); + if (dest instanceof JSONArray) { + ((JSONArray) dest).put(value); + return; + } + JSONArray arr = new JSONArray(); + arr.put(dest); + arr.put(value); + o.put(key, arr); + return; + } }