Bug 1348250 - Change TelemetryStorage to load crash pending pings outside of the profile. r?chutten draft
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Fri, 31 Mar 2017 12:01:32 +0200
changeset 554466 aa4192fe40ff99f24c6033d5b49abd211d2d18aa
parent 553822 8df9fabf2587b7020889755acb9e75b664fe13cf
child 622337 704adc57003ed1cbc3c80d50f67fbca1e28fc28c
push id51934
push useralessio.placitelli@gmail.com
push dateFri, 31 Mar 2017 15:57:08 +0000
reviewerschutten
bugs1348250
milestone55.0a1
Bug 1348250 - Change TelemetryStorage to load crash pending pings outside of the profile. r?chutten MozReview-Commit-ID: 1kqdGgEwXvI
toolkit/components/telemetry/TelemetryStorage.jsm
toolkit/components/telemetry/tests/unit/test_MigratePendingPings.js
toolkit/components/telemetry/tests/unit/xpcshell.ini
--- a/toolkit/components/telemetry/TelemetryStorage.jsm
+++ b/toolkit/components/telemetry/TelemetryStorage.jsm
@@ -1449,16 +1449,65 @@ var TelemetryStorageImpl = {
           continue;
         }
       }
     } finally {
       await iter.close();
     }
   },
 
+  /**
+   * This function migrates pings that are stored in the userApplicationDataDir
+   * under the "Pending Pings" sub-directory.
+   */
+  async _migrateAppDataPings() {
+    this._log.trace("_migrateAppDataPings");
+
+    // The tests suites might not create and define the "UAppData" directory.
+    // We account for that here instead of manually going through each test using
+    // telemetry to manually create the directory and define the constant.
+    if (!OS.Constants.Path.userApplicationDataDir) {
+      this._log.trace("_migrateAppDataPings - userApplicationDataDir is not defined. Is this a test?");
+      return;
+    }
+
+    const appDataPendingPings =
+      OS.Path.join(OS.Constants.Path.userApplicationDataDir, "Pending Pings");
+
+    // Iterate through the pending ping files.
+    let iter = new OS.File.DirectoryIterator(appDataPendingPings);
+    try {
+      // Check if appDataPendingPings exists and bail out if it doesn't.
+      if (!(await iter.exists())) {
+        this._log.trace("_migrateAppDataPings - the AppData pending pings directory doesn't exist.");
+        return;
+      }
+
+      let files = (await iter.nextBatch()).filter(e => !e.isDir);
+      for (let file of files) {
+        try {
+          // Load the ping data from the original file.
+          const pingData = await this.loadPingFile(file.path);
+
+          // Save it among the pending pings in the user profile, overwrite on
+          // ping id collision.
+          await TelemetryStorage.savePing(pingData, true);
+
+          // Finally remove the file.
+          await OS.File.remove(file.path);
+        } catch (ex) {
+          this._log.error("_migrateAppDataPings - failed to remove file " + file.path, ex);
+          continue;
+        }
+      }
+    } finally {
+      await iter.close();
+    }
+  },
+
   loadPendingPingList() {
     // If we already have a pending scanning task active, return that.
     if (this._scanPendingPingsTask) {
       return this._scanPendingPingsTask;
     }
 
     if (this._scannedPendingDirectory) {
       this._log.trace("loadPendingPingList - Pending already scanned, hitting cache.");
@@ -1479,16 +1528,20 @@ var TelemetryStorageImpl = {
 
   getPendingPingList() {
     return this._buildPingList();
   },
 
   async _scanPendingPings() {
     this._log.trace("_scanPendingPings");
 
+    // Before pruning the pending pings, migrate over the ones from the user
+    // application data directory (mainly crash pings that failed to be sent).
+    await this._migrateAppDataPings();
+
     let directory = TelemetryStorage.pingDirectoryPath;
     let iter = new OS.File.DirectoryIterator(directory);
     let exists = await iter.exists();
 
     try {
       if (!exists) {
         return [];
       }
new file mode 100644
--- /dev/null
+++ b/toolkit/components/telemetry/tests/unit/test_MigratePendingPings.js
@@ -0,0 +1,96 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/
+*/
+
+"use strict";
+
+Cu.import("resource://gre/modules/osfile.jsm", this);
+Cu.import("resource://gre/modules/Services.jsm", this);
+Cu.import("resource://gre/modules/TelemetryStorage.jsm", this);
+Cu.import("resource://gre/modules/TelemetryUtils.jsm", this);
+Cu.import("resource://testing-common/AppData.jsm", this);
+
+// The name of the pending pings directory outside of the user profile,
+// in the user app data directory.
+const PENDING_PING_DIR_NAME = "Pending Pings";
+
+async function createFakeAppDir() {
+  // Create a directory inside the profile and register it as UAppData, so
+  // we can stick fake crash pings inside there. We put it inside the profile
+  // just because we know that will get cleaned up after the test runs.
+  let profileDir = Services.dirsvc.get("ProfD", Ci.nsIFile);
+
+  // Create "<profile>/UAppData/Pending Pings".
+  const pendingPingsPath = OS.Path.join(profileDir.path, "UAppData", PENDING_PING_DIR_NAME);
+  await OS.File.makeDir(pendingPingsPath, { ignoreExisting: true,
+                                            from: OS.Constants.Path.profileDir });
+
+  await makeFakeAppDir();
+}
+
+add_task(function* setup() {
+  // Init the profile.
+  do_get_profile();
+  yield createFakeAppDir();
+  // Make sure we don't generate unexpected pings due to pref changes.
+  yield setEmptyPrefWatchlist();
+});
+
+add_task(function* test_migrateUnsentPings() {
+  const PINGS = [
+    {
+      type: "crash",
+      id: TelemetryUtils.generateUUID(),
+      payload: { foo: "bar"},
+      dateCreated: new Date(2010, 1, 1, 10, 0, 0),
+    },
+    {
+      type: "other",
+      id: TelemetryUtils.generateUUID(),
+      payload: { moo: "meh"},
+      dateCreated: new Date(2010, 2, 1, 10, 2, 0),
+    },
+  ];
+  const APP_DATA_DIR = Services.dirsvc.get("UAppData", Ci.nsIFile).path;
+  const APPDATA_PINGS_DIR = OS.Path.join(APP_DATA_DIR, PENDING_PING_DIR_NAME);
+
+  // Create some pending pings outside of the user profile.
+  for (let ping of PINGS) {
+    const pingPath = OS.Path.join(APPDATA_PINGS_DIR, ping.id + ".json");
+    yield TelemetryStorage.savePingToFile(ping, pingPath, true);
+  }
+
+  // Make sure the pending ping list is empty.
+  yield TelemetryStorage.testClearPendingPings();
+
+  // Start the migration from TelemetryStorage.
+  let pendingPings = yield TelemetryStorage.loadPendingPingList();
+  Assert.equal(pendingPings.length, 2,
+               "TelemetryStorage must have migrated 2 pings.");
+
+  for (let ping of PINGS) {
+    // Verify that the pings were migrated and are among the pending pings.
+    Assert.ok(pendingPings.find(p => p.id == ping.id),
+              "The ping must have been migrated.");
+
+    // Try to load the migrated ping from the user profile.
+    let migratedPing = yield TelemetryStorage.loadPendingPing(ping.id);
+    Assert.equal(ping.id, migratedPing.id, "Should have loaded the correct ping id.");
+    Assert.equal(ping.type, migratedPing.type,
+                 "Should have loaded the correct ping type.");
+    Assert.deepEqual(ping.payload, migratedPing.payload,
+                     "Should have loaded the correct payload.");
+
+    // Verify that the pings are no longer outside of the user profile.
+    const pingPath = OS.Path.join(APPDATA_PINGS_DIR, ping.id + ".json");
+    Assert.ok(!(yield OS.File.exists(pingPath)),
+              "The ping should not be in the Pending Pings directory anymore.");
+  }
+
+  // Delete the UAppData directory and make sure nothing breaks.
+  yield OS.File.removeDir(APP_DATA_DIR, {ignorePermissions: true});
+  Assert.ok(!(yield OS.File.exists(APP_DATA_DIR)),
+            "The UAppData directory must not exist anymore.");
+  TelemetryStorage.reset();
+  yield TelemetryStorage.loadPendingPingList();
+});
--- a/toolkit/components/telemetry/tests/unit/xpcshell.ini
+++ b/toolkit/components/telemetry/tests/unit/xpcshell.ini
@@ -23,16 +23,17 @@ generated-files =
   dictionary.xpi
   experiment.xpi
   extension.xpi
   extension-2.xpi
   system.xpi
   restartless.xpi
   theme.xpi
 
+[test_MigratePendingPings.js]
 [test_TelemetryHistograms.js]
 [test_SubsessionChaining.js]
 tags = addons
 [test_TelemetryEnvironment.js]
 skip-if = os == "android"
 tags = addons
 [test_PingAPI.js]
 skip-if = os == "android"