Bug 1270213 - Factor lockAndReadJSONFromFile out from getAllPings. r=sebastian r=sebastian draft
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 05 May 2016 14:37:02 -0700
changeset 364562 e9a8a8a71338d4326a819117cf463f1dcb56bdde
parent 364561 eeae7245ce2e4ef7c70c92dd913709dce77811f9
child 364563 51271da24968ce2e6e7653c430dcaa5373b4f4a9
push id17492
push usermichael.l.comella@gmail.com
push dateFri, 06 May 2016 21:43:40 +0000
reviewerssebastian, sebastian
bugs1270213
milestone49.0a1
Bug 1270213 - Factor lockAndReadJSONFromFile out from getAllPings. r=sebastian r=sebastian MozReview-Commit-ID: HtUkO0n50px
mobile/android/base/java/org/mozilla/gecko/telemetry/stores/TelemetryJSONFilePingStore.java
--- a/mobile/android/base/java/org/mozilla/gecko/telemetry/stores/TelemetryJSONFilePingStore.java
+++ b/mobile/android/base/java/org/mozilla/gecko/telemetry/stores/TelemetryJSONFilePingStore.java
@@ -140,36 +140,19 @@ public class TelemetryJSONFilePingStore 
         }
     }
 
     @Override
     public ArrayList<TelemetryPing> getAllPings() {
         final File[] files = storeDir.listFiles(new PingFileFilter());
         final ArrayList<TelemetryPing> out = new ArrayList<>(files.length);
         for (final File file : files) {
-            final FileInputStream inputStream;
-            try {
-                inputStream = new FileInputStream(file);
-            } catch (final FileNotFoundException e) {
-                throw new IllegalStateException("Expected file to exist");
-            }
-
-            final JSONObject obj;
-            try {
-                // Potential optimization: re-use the same buffer for reading from files.
-                obj = lockAndReadFileAndCloseStream(inputStream, (int) file.length());
-            } catch (final IOException | JSONException e) {
-                // We couldn't read this file so let's just skip it. These potentially
-                // corrupted files should be removed when the data is pruned.
-                Log.w(LOGTAG, "Error when reading file: " + file.getName() + " Likely corrupted. Ignoring");
-                continue;
-            }
-
+            final JSONObject obj = lockAndReadJSONFromFile(file);
             if (obj == null) {
-                Log.d(LOGTAG, "Could not read given file: " + file.getName() + " File is locked. Ignoring");
+                // We log in the method to get the JSONObject if we return null.
                 continue;
             }
 
             try {
                 final String url = obj.getString(KEY_URL_PATH);
                 final ExtendedJSONObject payload = new ExtendedJSONObject(obj.getString(KEY_PAYLOAD));
                 final int id = getIDFromFilename(file.getName());
                 if (id < 0) {
@@ -180,16 +163,46 @@ public class TelemetryJSONFilePingStore 
             } catch (final IOException | JSONException | NonObjectJSONException e) {
                 Log.w(LOGTAG, "Bad json in ping. Ignoring.");
                 continue;
             }
         }
         return out;
     }
 
+    /**
+     * Logs if there is an error.
+     *
+     * @return the JSON object from the given file or null if there is an error.
+     */
+    private JSONObject lockAndReadJSONFromFile(final File file) {
+        final FileInputStream inputStream;
+        try {
+            inputStream = new FileInputStream(file);
+        } catch (final FileNotFoundException e) {
+            throw new IllegalStateException("Expected file to exist");
+        }
+
+        final JSONObject obj;
+        try {
+            // Potential optimization: re-use the same buffer for reading from files.
+            obj = lockAndReadFileAndCloseStream(inputStream, (int) file.length());
+        } catch (final IOException | JSONException e) {
+            // We couldn't read this file so let's just skip it. These potentially
+            // corrupted files should be removed when the data is pruned.
+            Log.w(LOGTAG, "Error when reading file: " + file.getName() + " Likely corrupted. Ignoring");
+            return null;
+        }
+
+        if (obj == null) {
+            Log.d(LOGTAG, "Could not read given file: " + file.getName() + " File is locked. Ignoring");
+        }
+        return obj;
+    }
+
     @Override
     public void onUploadAttemptComplete(final Set<Integer> successfulRemoveIDs) {
         if (successfulRemoveIDs.isEmpty()) {
             return;
         }
 
         final File[] files = storeDir.listFiles(new PingFileFilter(successfulRemoveIDs));
         for (final File file : files) {