Bug 1249596 - Part 2: Remove FHR client id migration code. r?dexter
Removed a fallback import from a legacy FHR file when there is no valid ID in the DRS file.
This commit is related to
bug 1431544
MozReview-Commit-ID: AACq7InWJpy
--- a/browser/components/migration/FirefoxProfileMigrator.js
+++ b/browser/components/migration/FirefoxProfileMigrator.js
@@ -239,52 +239,32 @@ FirefoxProfileMigrator.prototype._getRes
let createSubDir = (name) => {
let dir = currentProfileDir.clone();
dir.append(name);
dir.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
return dir;
};
// If the 'datareporting' directory exists we migrate files from it.
- let haveStateFile = false;
let dataReportingDir = this._getFileObject(sourceProfileDir, "datareporting");
if (dataReportingDir && dataReportingDir.isDirectory()) {
// Copy only specific files.
let toCopy = ["state.json", "session-state.json"];
let dest = createSubDir("datareporting");
let enumerator = dataReportingDir.directoryEntries;
while (enumerator.hasMoreElements()) {
let file = enumerator.getNext().QueryInterface(Ci.nsIFile);
if (file.isDirectory() || toCopy.indexOf(file.leafName) == -1) {
continue;
}
-
- if (file.leafName == "state.json") {
- haveStateFile = true;
- }
file.copyTo(dest, "");
}
}
- if (!haveStateFile) {
- // Fall back to migrating the state file that contains the client id from healthreport/.
- // We first moved the client id management from the FHR implementation to the datareporting
- // service.
- // Consequently, we try to migrate an existing FHR state file here as a fallback.
- let healthReportDir = this._getFileObject(sourceProfileDir, "healthreport");
- if (healthReportDir && healthReportDir.isDirectory()) {
- let stateFile = this._getFileObject(healthReportDir, "state.json");
- if (stateFile) {
- let dest = createSubDir("healthreport");
- stateFile.copyTo(dest, "");
- }
- }
- }
-
aCallback(true);
},
};
return [places, cookies, passwords, formData, dictionary, bookmarksBackups,
session, sync, times, telemetry, favicons].filter(r => r);
};
--- a/browser/components/migration/tests/unit/test_fx_telemetry.js
+++ b/browser/components/migration/tests/unit/test_fx_telemetry.js
@@ -136,39 +136,16 @@ add_task(async function test_migrate_fil
checkDirectoryContains(targetDir, {
"datareporting": {
"state.json": stateContent,
"session-state.json": sessionStateContent,
},
});
});
-add_task(async function test_fallback_fhr_state() {
- let [srcDir, targetDir] = getTestDirs();
-
- // Test that we fall back to migrating FHR state if the datareporting
- // state file does not exist.
- let stateContent = JSON.stringify({
- clientId: "68d5474e-19dc-45c1-8e9a-81fca592707c",
- });
- let subDir = createSubDir(srcDir, "healthreport");
- writeToFile(subDir, "state.json", stateContent);
-
- // Perform migration.
- let ok = await promiseTelemetryMigrator(srcDir, targetDir);
- Assert.ok(ok, "callback should have been true");
-
- checkDirectoryContains(targetDir, {
- "healthreport": {
- "state.json": stateContent,
- },
- });
-});
-
-
add_task(async function test_datareporting_not_dir() {
let [srcDir, targetDir] = getTestDirs();
writeToFile(srcDir, "datareporting", "I'm a file but should be a directory");
let ok = await promiseTelemetryMigrator(srcDir, targetDir);
Assert.ok(ok, "callback should have been true even though the directory was a file");
--- a/toolkit/modules/ClientID.jsm
+++ b/toolkit/modules/ClientID.jsm
@@ -40,18 +40,17 @@ const PREF_CACHED_CLIENTID = "toolkit.te
function isValidClientID(id) {
const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
return UUID_REGEX.test(id);
}
this.ClientID = Object.freeze({
/**
* This returns a promise resolving to the the stable client ID we use for
- * data reporting (FHR & Telemetry). Previously exising FHR client IDs are
- * migrated to this.
+ * data reporting (FHR & Telemetry).
*
* WARNING: This functionality is duplicated for Android (see GeckoProfile.getClientId
* for more). There are Java tests (TestGeckoProfile) to ensure the functionality is
* consistent and Gecko tests to come (bug 1249156). However, THIS IS NOT FOOLPROOF.
* Be careful when changing this code and, in particular, the underlying file format.
*
* @return {Promise<string>} The stable client ID.
*/
@@ -91,45 +90,32 @@ var ClientIDImpl = {
}
this._loadClientIdTask = this._doLoadClientID();
let clear = () => this._loadClientIdTask = null;
this._loadClientIdTask.then(clear, clear);
return this._loadClientIdTask;
},
+ /**
+ * Load the Client ID from the DataReporting Service state file.
+ * If no Client ID is found, we generate a new one.
+ */
async _doLoadClientID() {
- // As we want to correlate FHR and telemetry data (and move towards unifying the two),
- // we first moved the ID management from the FHR implementation to the datareporting
- // service, then to a common shared module.
- // Consequently, we try to import an existing FHR ID, so we can keep using it.
-
- // Try to load the client id from the DRS state file first.
+ // Try to load the client id from the DRS state file.
try {
let state = await CommonUtils.readJSON(gStateFilePath);
if (state && this.updateClientID(state.clientID)) {
return this._clientID;
}
} catch (e) {
// fall through to next option
}
- // If we dont have DRS state yet, try to import from the FHR state.
- try {
- let fhrStatePath = OS.Path.join(OS.Constants.Path.profileDir, "healthreport", "state.json");
- let state = await CommonUtils.readJSON(fhrStatePath);
- if (state && this.updateClientID(state.clientID)) {
- this._saveClientID();
- return this._clientID;
- }
- } catch (e) {
- // fall through to next option
- }
-
- // We dont have an id from FHR yet, generate a new ID.
+ // We dont have an id from the DRS state file yet, generate a new ID.
this.updateClientID(CommonUtils.generateUUID());
this._saveClientIdTask = this._saveClientID();
// Wait on persisting the id. Otherwise failure to save the ID would result in
// the client creating and subsequently sending multiple IDs to the server.
// This would appear as multiple clients submitting similar data, which would
// result in orphaning.
await this._saveClientIdTask;
@@ -146,18 +132,17 @@ var ClientIDImpl = {
let obj = { clientID: this._clientID };
await OS.File.makeDir(gDatareportingPath);
await CommonUtils.writeJSON(obj, gStateFilePath);
this._saveClientIdTask = null;
},
/**
* This returns a promise resolving to the the stable client ID we use for
- * data reporting (FHR & Telemetry). Previously exising FHR client IDs are
- * migrated to this.
+ * data reporting (FHR & Telemetry).
*
* @return {Promise<string>} The stable client ID.
*/
getClientID() {
if (!this._clientID) {
return this._loadClientID();
}
--- a/toolkit/modules/tests/xpcshell/test_client_id.js
+++ b/toolkit/modules/tests/xpcshell/test_client_id.js
@@ -12,81 +12,43 @@ ChromeUtils.import("resource://gre/modul
function run_test() {
do_get_profile();
run_next_test();
}
add_task(async function() {
const drsPath = OS.Path.join(OS.Constants.Path.profileDir, "datareporting", "state.json");
- const fhrDir = OS.Path.join(OS.Constants.Path.profileDir, "healthreport");
- const fhrPath = OS.Path.join(fhrDir, "state.json");
const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
const invalidIDs = [
[-1, "setIntPref"],
[0.5, "setIntPref"],
["INVALID-UUID", "setStringPref"],
[true, "setBoolPref"],
["", "setStringPref"],
["3d1e1560-682a-4043-8cf2-aaaaaaaaaaaZ", "setStringPref"],
];
const PREF_CACHED_CLIENTID = "toolkit.telemetry.cachedClientID";
- await OS.File.makeDir(fhrDir);
-
- // Check that we are importing the FHR client ID.
- let clientID = CommonUtils.generateUUID();
- await CommonUtils.writeJSON({clientID}, fhrPath);
- Assert.equal(clientID, await ClientID.getClientID());
-
- // We should persist the ID in DRS now and not pick up a differing ID from FHR.
+ // If there is no DRS file, we should get a new client ID.
await ClientID._reset();
- await CommonUtils.writeJSON({clientID: CommonUtils.generateUUID()}, fhrPath);
- Assert.equal(clientID, await ClientID.getClientID());
-
- // We should be guarded against broken FHR data.
- for (let invalidID of invalidIDs) {
- await ClientID._reset();
- await OS.File.remove(drsPath);
- await CommonUtils.writeJSON({clientID: invalidID}, fhrPath);
- clientID = await ClientID.getClientID();
- Assert.equal(typeof(clientID), "string");
- Assert.ok(uuidRegex.test(clientID));
- }
-
- // We should be guarded against invalid FHR json.
- await ClientID._reset();
- await OS.File.remove(drsPath);
- await OS.File.writeAtomic(fhrPath, "abcd", {encoding: "utf-8", tmpPath: fhrPath + ".tmp"});
- clientID = await ClientID.getClientID();
+ let clientID = await ClientID.getClientID();
Assert.equal(typeof(clientID), "string");
Assert.ok(uuidRegex.test(clientID));
- // We should be guarded against broken DRS data too and fall back to loading
- // the FHR ID.
- for (let invalidID of invalidIDs) {
- await ClientID._reset();
- clientID = CommonUtils.generateUUID();
- await CommonUtils.writeJSON({clientID}, fhrPath);
- await CommonUtils.writeJSON({clientID: invalidID}, drsPath);
- Assert.equal(clientID, await ClientID.getClientID());
- }
-
- // We should be guarded against invalid DRS json too.
+ // We should be guarded against invalid DRS json.
await ClientID._reset();
- await OS.File.remove(fhrPath);
await OS.File.writeAtomic(drsPath, "abcd", {encoding: "utf-8", tmpPath: drsPath + ".tmp"});
clientID = await ClientID.getClientID();
Assert.equal(typeof(clientID), "string");
Assert.ok(uuidRegex.test(clientID));
- // If both the FHR and DSR data are broken, we should end up with a new client ID.
+ // If the DRS data is broken, we should end up with a new client ID.
for (let [invalidID, ] of invalidIDs) {
await ClientID._reset();
- await CommonUtils.writeJSON({clientID: invalidID}, fhrPath);
await CommonUtils.writeJSON({clientID: invalidID}, drsPath);
clientID = await ClientID.getClientID();
Assert.equal(typeof(clientID), "string");
Assert.ok(uuidRegex.test(clientID));
}
// Assure that cached IDs are being checked for validity.
for (let [invalidID, prefFunc] of invalidIDs) {