Bug 1354016 - Forms validator ignore clientMissing. r=tcsc
authortiago <tiago.paez11@gmail.com>
Thu, 01 Jun 2017 22:32:23 -0300
changeset 412887 be7694b72c8a3d5de5451901d1bc268cf73021a2
parent 412886 3050cd0ed9eecfe47c42d0dbf5b61e4cdffff53d
child 412888 80c6e635835034ae5953a26fe5edb8aba060848f
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcsc
bugs1354016
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1354016 - Forms validator ignore clientMissing. r=tcsc MozReview-Commit-ID: C2FD58C9HzS
services/sync/modules/collection_validator.js
services/sync/modules/engines/forms.js
services/sync/tests/unit/test_form_validator.js
services/sync/tests/unit/xpcshell.ini
--- a/services/sync/modules/collection_validator.js
+++ b/services/sync/modules/collection_validator.js
@@ -47,16 +47,22 @@ class CollectionValidator {
   // - idProp: Property that identifies a record. That is, if a client and server
   //   record have the same value for the idProp property, they should be
   //   compared against eachother.
   // - props: Array of properties that should be compared
   constructor(name, idProp, props) {
     this.name = name;
     this.props = props;
     this.idProp = idProp;
+
+    // This property deals with the fact that form history records are never
+    // deleted from the server. The FormValidator subclass needs to ignore the
+    // client missing records, and it uses this property to achieve it -
+    // (Bug 1354016).
+    this.ignoresMissingClients = false;
   }
 
   // Should a custom ProblemData type be needed, return it here.
   emptyProblemData() {
     return new CollectionProblemData();
   }
 
   async getServerItems(engine) {
@@ -172,17 +178,17 @@ class CollectionValidator {
         allRecords.set(id, { client: record, server: null });
       }
     }
 
     for (let [id, { server, client }] of allRecords) {
       if (!client && !server) {
         throw new Error("Impossible: no client or server record for " + id);
       } else if (server && !client) {
-        if (server.understood) {
+        if (!this.ignoresMissingClients && server.understood) {
           problems.clientMissing.push(id);
         }
       } else if (client && !server) {
         if (client.shouldSync) {
           problems.serverMissing.push(id);
         }
       } else {
         if (!client.shouldSync) {
--- a/services/sync/modules/engines/forms.js
+++ b/services/sync/modules/engines/forms.js
@@ -261,16 +261,17 @@ class FormsProblemData extends Collectio
     return super.getSummary().filter(entry =>
       entry.name !== "clientMissing");
   }
 }
 
 class FormValidator extends CollectionValidator {
   constructor() {
     super("forms", "id", ["name", "value"]);
+    this.ignoresMissingClients = true;
   }
 
   emptyProblemData() {
     return new FormsProblemData();
   }
 
   getClientItems() {
     return FormWrapper._promiseSearch(["guid", "fieldname", "value"], {});
new file mode 100644
--- /dev/null
+++ b/services/sync/tests/unit/test_form_validator.js
@@ -0,0 +1,93 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Components.utils.import("resource://services-sync/engines/forms.js");
+
+function getDummyServerAndClient() {
+  return {
+    server: [
+      {
+        id: "11111",
+        guid: "11111",
+        name: "foo",
+        fieldname: "foo",
+        value: "bar",
+      },
+      {
+        id: "22222",
+        guid: "22222",
+        name: "foo2",
+        fieldname: "foo2",
+        value: "bar2",
+      },
+      {
+        id: "33333",
+        guid: "33333",
+        name: "foo3",
+        fieldname: "foo3",
+        value: "bar3",
+      },
+    ],
+    client: [
+      {
+        id: "11111",
+        guid: "11111",
+        name: "foo",
+        fieldname: "foo",
+        value: "bar",
+      },
+      {
+        id: "22222",
+        guid: "22222",
+        name: "foo2",
+        fieldname: "foo2",
+        value: "bar2",
+      },
+      {
+        id: "33333",
+        guid: "33333",
+        name: "foo3",
+        fieldname: "foo3",
+        value: "bar3",
+      }
+    ]
+  };
+}
+
+add_test(function test_valid() {
+  let { server, client } = getDummyServerAndClient();
+  let validator = new FormValidator();
+  let { problemData, clientRecords, records, deletedRecords } =
+      validator.compareClientWithServer(client, server);
+  equal(clientRecords.length, 3);
+  equal(records.length, 3)
+  equal(deletedRecords.length, 0);
+  deepEqual(problemData, validator.emptyProblemData());
+
+  run_next_test();
+});
+
+
+add_test(function test_formValidatorIgnoresMissingClients() {
+  // Since history form records are not deleted from the server, the
+  // |FormValidator| shouldn't set the |missingClient| flag in |problemData|.
+  let { server, client } = getDummyServerAndClient();
+  client.pop();
+
+  let validator = new FormValidator();
+  let { problemData, clientRecords, records, deletedRecords } =
+      validator.compareClientWithServer(client, server);
+
+  equal(clientRecords.length, 2);
+  equal(records.length, 3);
+  equal(deletedRecords.length, 0);
+
+  let expected = validator.emptyProblemData();
+  deepEqual(problemData, expected);
+
+  run_next_test();
+});
+
+function run_test() {
+  run_next_test();
+}
--- a/services/sync/tests/unit/xpcshell.ini
+++ b/services/sync/tests/unit/xpcshell.ini
@@ -153,16 +153,17 @@ requesttimeoutfactor = 4
 [test_clients_escape.js]
 [test_doctor.js]
 [test_extension_storage_engine.js]
 [test_extension_storage_tracker.js]
 [test_forms_store.js]
 [test_forms_tracker.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
+[test_form_validator.js]
 [test_history_store.js]
 [test_history_tracker.js]
 # Too many intermittent "ASSERTION: thread pool wasn't shutdown: '!mPool'" (bug 804479)
 skip-if = debug
 [test_places_guid_downgrade.js]
 [test_password_engine.js]
 [test_password_store.js]
 [test_password_validator.js]