FIX #76 deepEqual does not work correctly for Arrays

pubkey · Mar 12, 2017 · b55a751 · b55a751
1 parent 5dbe535
commit b55a751
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 105 deletions.
diff --git a/package.json b/package.json
@@ -57,6 +57,7 @@
     "babel-runtime": "^6.23.0",
     "clone": "2.1.0",
     "crypto-js": "3.1.8",
+    "deep-equal": "^1.0.1",
     "es6-promise": "^4.0.5",
     "is-my-json-valid": "^2.16.0",
     "object-path": "^0.11.4",

diff --git a/src/RxDocument.js b/src/RxDocument.js
@@ -4,6 +4,9 @@ import {
 import {
     default as objectPath
 } from 'object-path';
+import {
+    default as deepEqual
+} from 'deep-equal';
 
 import * as util from './util';
 import * as RxChangeEvent from './RxChangeEvent';
@@ -87,7 +90,7 @@ class RxDocument {
                 const prevSyncData = this._dataSync$.getValue();
                 const prevData = this._data;
 
-                if (isDeepEqual(prevSyncData, prevData)) {
+                if (deepEqual(prevSyncData, prevData)) {
                     // document is in sync, overwrite _data
                     this._data = newData;
 
@@ -256,7 +259,7 @@ class RxDocument {
             throw new Error('RxDocument.save(): cant save deleted document');
 
         // check if different
-        if (isDeepEqual(this._data, this._dataSync$.getValue())) {
+        if (deepEqual(this._data, this._dataSync$.getValue())) {
             this._synced$.next(true);
             return false; // nothing changed, dont save
         }
@@ -313,50 +316,6 @@ class RxDocument {
     destroy() {}
 }
 
-/**
- * performs a deep-equal without comparing internal getters and setter (observe$ and populate_ etc.)
- * @param  {object}  data1
- * @param  {object}  data2
- * @throws {Error} if given data not a plain js object
- * @return {Boolean} true if equal
- */
-export function isDeepEqual(data1, data2) {
-    if (typeof data1 !== typeof data2) return false;
-
-    let ret = true;
-
-    // array
-    if (Array.isArray(data1)) {
-        let k = 0;
-        while (k < data1.length && ret == true) {
-            if (!data2[k] || !isDeepEqual(data1[k], data2[k]))
-                ret = false;
-            k++;
-        }
-        return ret;
-    }
-
-    // object
-    if (typeof data1 === 'object') {
-        const entries = Object.entries(data1)
-            .filter(entry => !entry[0].endsWith('$')) // observe
-            .filter(entry => !entry[0].endsWith('_')); // populate;
-        let k = 0;
-        while (k < entries.length && ret) {
-            const entry = entries[k];
-            const name = entry[0];
-            const value = entry[1];
-            if (!isDeepEqual(data2[name], value))
-                ret = false;
-            k++;
-        }
-        return ret;
-    }
-
-    // other
-    return data1 == data2;
-}
-
 export function create(collection, jsonData) {
     if (jsonData[collection.schema.primaryPath].startsWith('_design'))
         return null;

diff --git a/test/helper/schemas.js b/test/helper/schemas.js
@@ -457,6 +457,27 @@ export const heroArray = {
     'required': ['color']
 };
 
+export const simpleArrayHero = {
+    title: 'hero schema',
+    version: 0,
+    description: 'describes a hero with a string-array-field',
+    type: 'object',
+    properties: {
+        name: {
+            type: 'string',
+            primary: true
+        },
+        skills: {
+            type: 'array',
+            maxItems: 5,
+            uniqueItems: true,
+            item: {
+                type: 'string',
+            }
+        }
+    }
+};
+
 
 export const primaryHuman = {
     title: 'human schema with primary',

diff --git a/test/unit/RxDocument.test.js b/test/unit/RxDocument.test.js
@@ -4,73 +4,17 @@ import * as _ from 'lodash';
 
 import * as humansCollection from './../helper/humans-collection';
 import * as schemaObjects from '../helper/schema-objects';
+import * as schemas from '../helper/schemas';
 import * as util from '../../dist/lib/util';
 import * as RxDocument from '../../dist/lib/RxDocument';
+import * as RxDatabase from '../../dist/lib/index';
 
 process.on('unhandledRejection', function(err) {
     throw err;
 });
 
 describe('RxDocument.test.js', () => {
-    describe('statics', () => {
-        describe('.isDeepEqual()', () => {
-            it('should true on standard object', () => {
-                const is = RxDocument.isDeepEqual({
-                    foo: 'baar'
-                }, {
-                    foo: 'baar'
-                });
-                assert.ok(is);
-            });
-            it('should false on standard object', () => {
-                const is = RxDocument.isDeepEqual({
-                    foo: 'baar'
-                }, {
-                    foo: 'baar1'
-                });
-                assert.equal(is, false);
-            });
-            it('should true on array', () => {
-                const is = RxDocument.isDeepEqual([{
-                    foo: 'baar'
-                }], [{
-                    foo: 'baar'
-                }]);
-                assert.ok(is);
-            });
-            it('should false on array', () => {
-                const is = RxDocument.isDeepEqual([{
-                    foo: 'baar'
-                }], [{
-                    foo: 'baar2'
-                }]);
-                assert.equal(is, false);
-            });
-            it('should true on getter', () => {
-                const obj1 = {};
-                obj1.__defineGetter__('foo', () => 'bar');
-                const obj2 = {};
-                obj2.__defineGetter__('foo', () => 'bar');
-                assert.ok(RxDocument.isDeepEqual(obj1, obj2));
-            });
-            it('should false on different getter', () => {
-                const obj1 = {};
-                obj1.__defineGetter__('foo', () => 'bar');
-                const obj2 = {};
-                obj2.__defineGetter__('foo', () => 'bar1');
-                assert.equal(RxDocument.isDeepEqual(obj1, obj2), false);
-            });
-            it('should ignore getter which endsWith $', () => {
-                const obj1 = {};
-                obj1.__defineGetter__('foo', () => 'bar');
-                obj1.__defineGetter__('foo$', () => 'bar');
-                const obj2 = {};
-                obj2.__defineGetter__('foo', () => 'bar');
-                assert.ok(RxDocument.isDeepEqual(obj1, obj2));
-            });
-
-        });
-    });
+    describe('statics', () => {});
     describe('.get()', () => {
         describe('positive', () => {
             it('get a value', async() => {
@@ -480,5 +424,35 @@ describe('RxDocument.test.js', () => {
 
             c.database.destroy();
         });
+        it('BUG #76 - deepEqual does not work correctly for Arrays', async() => {
+            const db = await RxDatabase.create({
+                name: util.randomCouchString(10),
+                adapter: 'memory'
+            });
+            const col = await await db.collection({
+                name: 'heroes',
+                schema: schemas.simpleArrayHero
+            });
+            const docData = {
+                name: 'foobar',
+                skills: [
+                    'skill1',
+                    'skill2',
+                    'skill3'
+                ]
+            };
+            await col.insert(docData);
+            const doc = await col.findOne().exec();
+            assert.equal(doc.skills.length, 3);
+
+            const newSkill = 'newSikSkill';
+            doc.set('skills', doc.skills.concat(newSkill));
+            await doc.save();
+
+            const colDump = await col.dump(true);
+            const afterSkills = colDump.docs[0].skills;
+            assert.equal(afterSkills.length, 4);
+            assert.ok(afterSkills.includes(newSkill));
+        });
     });
 });