Fix SyncResultObject performances using a Map (fixes #806) (#807)

* Fix SyncResultObject performances using a Set (fixes #806)

* Restore previous behaviour regarding duplicates

* Fix tests

* Use Map instead of Set

* Add test suggested by @glasserc, which passes on master

* Merge everything into the Map

This solution has different ordering properties than the original one,
but that seems OK since I feel that all of these arrays are really
unordered collections (i.e. Sets).

src/collection.js

@@ -72,15 +72,25 @@ export class SyncResultObject {
     if (!Array.isArray(this[type])) {
       return;
     }
+    if (!Array.isArray(entries)) {
+      entries = [entries];
+    }
     // Deduplicate entries by id. If the values don't have `id` attribute, just
     // keep all.
-    const deduplicated = this[type].concat(entries).reduce((acc, cur) => {
-      const existing = acc.filter(
-        r => (cur.id && r.id ? cur.id != r.id : true)
-      );
-      return existing.concat(cur);
-    }, []);
-    this[type] = deduplicated;
+    const recordsWithoutId = new Set(this[type].filter(record => !record.id));
+    const recordsById = new Map(
+      this[type].filter(record => record.id).map(record => [record.id, record])
+    );
+    entries.forEach(record => {
+      if (!record.id) {
+        recordsWithoutId.add(record);
+      } else {
+        recordsById.set(record.id, record);
+      }
+    });
+    this[type] = Array.from(recordsById.values()).concat(
+      Array.from(recordsWithoutId)
+    );
     this.ok = this.errors.length + this.conflicts.length === 0;
     return this;
   }

test/collection_test.js

@@ -363,6 +363,29 @@ describe("Collection", () => {
         expect(result.skipped).eql([1, 2, 3]);
       });
 
+      it("should overwrite entries with same id", () => {
+        const result = new SyncResultObject();
+
+        result.add("skipped", [{ id: 1, name: "a" }]);
+        result.add("skipped", [{ id: 2, name: "b" }]);
+        result.add("skipped", [{ id: 1, name: "c" }]);
+        result.add("skipped", [{ name: "d" }]);
+        result.add("skipped", [{ name: "e" }]);
+        expect(result.skipped).eql([
+          { id: 1, name: "c" },
+          { id: 2, name: "b" },
+          { name: "d" },
+          { name: "e" },
+        ]);
+      });
+
+      it("should deduplicate added entries with same id", () => {
+        const result = new SyncResultObject();
+
+        result.add("created", [{ id: 1, name: "a" }, { id: 1, name: "b" }]);
+        expect(result.created).eql([{ id: 1, name: "b" }]);
+      });
+
       it("should update the ok status flag on errors", () => {
         const result = new SyncResultObject();
 
@@ -392,6 +415,17 @@ describe("Collection", () => {
 
         expect(result.add("resolved", [])).eql(result);
       });
+
+      it("should support adding single objects", () => {
+        const result = new SyncResultObject();
+
+        const e = {
+          type: "incoming",
+          message: "conflict",
+        };
+        result.add("errors", e);
+        expect(result.errors).eql([e]);
+      });
     });
 
     /** @test {SyncResultObject#reset} */