js-data · pik · Aug 2, 2016 · Aug 2, 2016 · Aug 2, 2016 · Aug 2, 2016
diff --git a/src/DataStore.js b/src/DataStore.js
@@ -1,4 +1,5 @@
-import utils from './utils'
+import utils, { safeSetLink, safeSetProp } from './utils'
+
 import {
   belongsToType,
   hasManyType,
@@ -261,22 +262,6 @@ const ownMethodsForScoping = [
   'hashQuery'
 ]
 
-const safeSetProp = function (record, field, value) {
-  if (record && record._set) {
-    record._set(`props.${field}`, value)
-  } else {
-    utils.set(record, field, value)
-  }
-}
-
-const safeSetLink = function (record, field, value) {
-  if (record && record._set) {
-    record._set(`links.${field}`, value)
-  } else {
-    utils.set(record, field, value)
-  }
-}
-
 const cachedFn = function (name, hashOrId, opts) {
   const cached = this._completedQueries[name][hashOrId]
   if (utils.isFunction(cached)) {
@@ -1013,7 +998,6 @@ const props = {
     })
 
     const idAttribute = mapper.idAttribute
-
     mapper.relationList.forEach(function (def) {
       const relation = def.relation
       const localField = def.localField
@@ -1046,19 +1030,8 @@ const props = {
 
             // e.g. profile.user !== someUser
             // or comment.post !== somePost
-            if (currentParent) {
-              // e.g. otherUser.profile = undefined
-              if (inverseDef.type === hasOneType) {
-                safeSetLink(currentParent, inverseDef.localField, undefined)
-              } else if (inverseDef.type === hasManyType) {
-                // e.g. remove comment from otherPost.comments
-                const children = utils.get(currentParent, inverseDef.localField)
-                if (id === undefined) {
-                  utils.remove(children, (child) => child === this)
-                } else {
-                  utils.remove(children, (child) => child === this || id === utils.get(child, idAttribute))
-                }
-              }
+            if (currentParent && inverseDef) {
+              this.removeInverseRelation(currentParent, id, inverseDef, idAttribute)
             }
             if (record) {
               // e.g. profile.user = someUser
@@ -1077,18 +1050,8 @@ const props = {
               safeSetProp(this, foreignKey, relatedId)
               collection.updateIndex(this, updateOpts)
 
-              // Update (set) inverse relation
-              if (inverseDef.type === hasOneType) {
-                // e.g. someUser.profile = profile
-                safeSetLink(record, inverseDef.localField, this)
-              } else if (inverseDef.type === hasManyType) {
-                // e.g. add comment to somePost.comments
-                const children = utils.get(record, inverseDef.localField)
-                if (id === undefined) {
-                  utils.noDupeAdd(children, this, (child) => child === this)
-                } else {
-                  utils.noDupeAdd(children, this, (child) => child === this || id === utils.get(child, idAttribute))
-                }
+              if (inverseDef) {
+                this.setupInverseRelation(record, id, inverseDef, idAttribute)
               }
             } else {
               // Unset in-memory link only
@@ -1338,6 +1301,14 @@ const props = {
               safeSetLink(record, inverseLocalField, this)
             } else {
               // Unset locals
+              if (current) {
+                // Update (unset) inverse relation
+                /* Note that setting foreignKey to null / undefined breaks other specs
+                * Seems like there are other errors
+                */
+                // safeSetProp(current, foreignKey, undefined)
+                safeSetLink(current, inverseLocalField, undefined)
+              }
               safeSetLink(this, localField, undefined)
             }
             return record

diff --git a/src/Record.js b/src/Record.js
@@ -1,6 +1,11 @@
-import utils from './utils'
+import utils, { safeSetLink } from './utils'
 import Component from './Component'
 import Settable from './Settable'
+import {
+  belongsToType,
+  hasManyType,
+  hasOneType
+} from './decorators'
 
 const DOMAIN = 'Record'
 
@@ -386,6 +391,36 @@ export default Component.extend({
     return !this._mapper().validate(this, opts)
   },
 
+  removeInverseRelation(currentParent, id, inverseDef, idAttribute) {
+    if (inverseDef.type === hasOneType) {
+      safeSetLink(currentParent, inverseDef.localField, undefined)
+    } else if (inverseDef.type === hasManyType) {
+      // e.g. remove comment from otherPost.comments
+      const children = utils.get(currentParent, inverseDef.localField)
+      if (id === undefined) {
+        utils.remove(children, (child) => child === this)
+      } else {
+        utils.remove(children, (child) => child === this || id === utils.get(child, idAttribute))
+      }
+    }
+  },
+
+  setupInverseRelation(record, id, inverseDef, idAttribute) {
+    // Update (set) inverse relation
+    if (inverseDef.type === hasOneType) {
+      // e.g. someUser.profile = profile
+      safeSetLink(record, inverseDef.localField, this)
+    } else if (inverseDef.type === hasManyType) {
+      // e.g. add comment to somePost.comments
+      const children = utils.get(record, inverseDef.localField)
+      if (id === undefined) {
+        utils.noDupeAdd(children, this, (child) => child === this)
+      } else {
+        utils.noDupeAdd(children, this, (child) => child === this || id === utils.get(child, idAttribute))
+      }
+    }
+  },
+
   /**
    * Lazy load relations of this record, to be attached to the record once their
    * loaded.

diff --git a/src/Relation.js b/src/Relation.js
@@ -156,18 +156,14 @@ utils.addHiddenPropsToTarget(Relation.prototype, {
     const localField = this.localField
     records.forEach((record) => {
       const relatedData = utils.get(record, localField)
-      this.unlinkInverseRecords(relatedData)
+      /* CAN DEPRECATE? */
+      // this.unlinkInverseRecords(relatedData, record)
+
+      // descriptor.set will trigger removeInverseRelations
       utils.set(record, localField, undefined)
     })
   },
 
-  unlinkInverseRecords (record) {
-    if (!record) {
-      return
-    }
-    utils.set(record, this.getInverse(this.mapper).localField, undefined)
-  },
-
   linkRecord (record, relatedRecord) {
     const relatedId = utils.get(relatedRecord, this.mapper.idAttribute)
 

diff --git a/src/utils.js b/src/utils.js
@@ -1576,6 +1576,23 @@ http://www.js-data.io/v3.0/docs/errors#${code}`
     }
 
     object[last] = undefined
+  },
+
+}
+
+export const safeSetProp = function (record, field, value) {
+  if (record && record._set) {
+    record._set(`props.${field}`, value)
+  } else {
+    utils.set(record, field, value)
+  }
+}
+
+export const safeSetLink = function (record, field, value) {
+  if (record && record._set) {
+    record._set(`links.${field}`, value)
+  } else {
+    utils.set(record, field, value)
   }
 }
 

diff --git a/test/unit/datastore/destroy.test.js b/test/unit/datastore/destroy.test.js
@@ -63,12 +63,14 @@ describe('DataStore#destroy', function () {
 
     const user = this.store.add('user', this.data.user10)
     assert.strictEqual(this.store.get('profile', this.data.profile15.id).user, user)
+    // console.log("TEST USER", user, "\n")
     assert.strictEqual(this.store.get('organization', this.data.organization14.id).users[0], user)
     assert.strictEqual(this.store.get('comment', this.data.comment11.id).user, user)
     assert.strictEqual(this.store.get('comment', this.data.comment12.id).user, user)
     assert.strictEqual(this.store.get('comment', this.data.comment13.id).user, user)
 
     const result = await this.store.destroy('user', user.id)
+    // assert.isTrue(this.store.get('profile', this.data.profile15.id).user == user)
     assert(destroyCalled, 'Adapter#destroy should have been called')
     assert.equal(this.store.get('profile', this.data.profile15.id).user, undefined)
     assert.equal(this.store.get('organization', this.data.organization14.id).users[0], undefined)

diff --git a/test/unit/decorators/belongsTo.test.js b/test/unit/decorators/belongsTo.test.js
@@ -77,6 +77,26 @@ describe('JSData.belongsTo', function () {
     assert.strictEqual(bar2.foo, foo)
     assert.strictEqual(bar3.foo, foo)
   })
+
+  it ('should not create an inverseLink if no inverseRelationship is defined', function() {
+    const store = new JSData.DataStore()
+    store.defineMapper('foo', {
+    })
+    store.defineMapper('bar', {
+      relations: {
+        belongsTo: {
+          foo: {
+            localField: '_foo',
+            foreignKey: 'foo_id'
+          }
+        }
+      }
+    })
+    const foo = store.add('foo', { id: 1 })
+    const bar = store.add('bar', { id: 1, foo_id: 1 })
+    assert.strictEqual(bar._foo, foo)
+  })
+
   it('should add property accessors to prototype of target and allow relation re-assignment using customizations', function () {
     const store = new JSData.DataStore()
     store.defineMapper('foo', {

diff --git a/test/unit/decorators/hasMany.test.js b/test/unit/decorators/hasMany.test.js
@@ -202,4 +202,38 @@ describe('JSData.hasMany', function () {
     assert.equal(getCalled, 11)
     assert.equal(setCalled, 2)
   })
+  it('unlinks correctly in related structures when a record is removed', function() {
+    const store = new JSData.DataStore();
+    const mapperA = store.defineMapper('A', {
+      properties: {
+        parent: { type: 'boolean' }
+      },
+      relations:  {
+        hasMany: {
+          B: {
+            localField: 'b',
+            foreignKey: 'a_id'
+          }
+        }
+      }
+    });
+    const mapperB = store.defineMapper('B', {
+      relations:  {
+        belongsTo: {
+          A: {
+            localField: 'a',
+            foreignKey: 'a_id'
+          }
+        }
+      }
+    });
+
+    // We add two records, which are not linked
+    const aRecord = store.add('A', {id: 1, parent: true});
+
+    store.add('B', [{id: 1, a_id: 1}, {id: 2, a_id: 1}]);
+    assert.equal(aRecord.b.length, 2, '2 items linked as expected');
+    store.remove('B', 2);
+    assert.equal(aRecord.b.length, 1, 'expected 1 item to still be linked but got empty array');
+  })
 })