Preserve hasmany order when pushing child

addon/-private/system/relationships/state/belongs-to.js

@@ -24,10 +24,12 @@ export default class BelongsToRelationship extends Relationship {
   }
 
   setCanonicalRecordData(recordData) {
-    if (recordData) {
-      this.addCanonicalRecordData(recordData);
-    } else if (this.canonicalState) {
-      this.removeCanonicalRecordData(this.canonicalState);
+    if (recordData !== this.canonicalState) {
+      if (recordData) {
+        this.addCanonicalRecordData(recordData);
+      } else if (this.canonicalState) {
+        this.removeCanonicalRecordData(this.canonicalState);
+      }
     }
     this.flushCanonicalLater();
   }
@@ -51,14 +53,16 @@ export default class BelongsToRelationship extends Relationship {
       return;
     }
 
-    if (this.canonicalState) {
-      this.removeCanonicalRecordData(this.canonicalState);
-    }
+    if (this.canonicalState !== recordData) {
+      if (this.canonicalState) {
+        this.removeCanonicalRecordData(this.canonicalState);
+      }
 
-    this.canonicalState = recordData;
-    super.addCanonicalRecordData(recordData);
-    this.setHasAnyRelationshipData(true);
-    this.setRelationshipIsEmpty(false);
+      this.canonicalState = recordData;
+      super.addCanonicalRecordData(recordData);
+      this.setHasAnyRelationshipData(true);
+      this.setRelationshipIsEmpty(false);
+    }
   }
 
   inverseDidDematerialize() {
@@ -107,13 +111,15 @@ export default class BelongsToRelationship extends Relationship {
     // TODO Igor cleanup
     assertPolymorphicType(this.recordData, this.relationshipMeta, recordData, this.store);
 
-    if (this.inverseRecordData) {
-      this.removeRecordData(this.inverseRecordData);
-    }
+    if (this.inverseRecordData !== recordData) {
+      if (this.inverseRecordData) {
+        this.removeRecordData(this.inverseRecordData);
+      }
 
-    this.inverseRecordData = recordData;
-    super.addRecordData(recordData);
-    this.notifyBelongsToChange();
+      this.inverseRecordData = recordData;
+      super.addRecordData(recordData);
+      this.notifyBelongsToChange();
+    }
   }
 
   removeRecordDataFromOwn(recordData) {

addon/-private/system/relationships/state/has-many.js

@@ -34,6 +34,29 @@ export default class ManyRelationship extends Relationship {
     super.addCanonicalRecordData(recordData, idx);
   }
 
+  moveCanonicalRecordData(recordData, idx) {
+    if (!this.canonicalMembers.has(recordData)) {
+      this.addCanonicalRecordData(recordData, idx);
+      return;
+    }
+    if (idx === undefined) {
+      this.removeCanonicalRecordData(recordData);
+      this.addCanonicalRecordData(recordData, idx);
+      return;
+    }
+    let oldIndex = this.canonicalState.indexOf(recordData);
+    if (this.canonicalState.length > idx) {
+      if (oldIndex !== idx) {
+        this.canonicalState.splice(oldIndex, 1);
+        this.canonicalState.splice(idx, 0, recordData);
+      }
+    } else {
+      this.canonicalState.splice(oldIndex, 1);
+      this.canonicalState.push(recordData);
+    }
+    this.flushCanonicalLater();
+  }
+
   inverseDidDematerialize(inverseRecordData) {
     super.inverseDidDematerialize(inverseRecordData);
     if (this.isAsync) {
@@ -156,11 +179,20 @@ export default class ManyRelationship extends Relationship {
 
     this.removeCanonicalRecordDatas(recordDatasToRemove);
 
-    for (let i = 0, l = recordDatas.length; i < l; i++) {
+    const newLength = recordDatas.length;
+
+    // we've got rid of all the old ones that aren't in here any more
+    // so everything else is a move or an add
+    // move falls back to add if the entry is not present
+
+    for (let i = 0; i < newLength; i++) {
       let recordData = recordDatas[i];
-      this.removeCanonicalRecordData(recordData);
-      this.addCanonicalRecordData(recordData, i);
+      this.moveCanonicalRecordData(recordData, i);
     }
+
+    // not necessarily calling addCanonicalRecordData, so flush in case
+    // duplicates are ignored so this doesn't result in extra work
+    this.flushCanonicalLater();
   }
 
   setInitialRecordDatas(recordDatas) {

addon/-private/system/relationships/state/relationship.js

@@ -69,7 +69,7 @@ export default class Relationship {
     this.relationshipMeta = relationshipMeta;
     //This probably breaks for polymorphic relationship in complex scenarios, due to
     //multiple possible modelNames
-    this.inverseKeyForImplicit = this._tempModelName + this.key;
+    this.inverseKeyForImplicit = (this._tempModelName ? this._tempModelName : '') + this.key;
     this.meta = null;
     this.__inverseMeta = undefined;
 
@@ -342,7 +342,7 @@ export default class Relationship {
   addCanonicalRecordData(recordData, idx) {
     heimdall.increment(addCanonicalRecordData);
     if (!this.canonicalMembers.has(recordData)) {
-      this.canonicalMembers.add(recordData);
+      this.canonicalMembers.addWithIndex(recordData, idx);
       this.setupInverseRelationship(recordData);
     }
     this.flushCanonicalLater();

tests/integration/adapter/client-side-delete-test.js

@@ -38,68 +38,70 @@ test('client-side deleted records can be added back from an inverse', async func
     assert.ok(false, 'unreachable');
   };
 
-  let bookstore = this.store.push({
-    data: {
-      id: '1',
-      type: 'bookstore',
-      relationships: {
-        books: {
-          data: [
-            {
-              id: '1',
-              type: 'book',
-            },
-            {
-              id: '2',
-              type: 'book',
-            },
-          ],
-        },
-      },
-    },
-    included: [
-      {
+  return await run(async () => {
+    let bookstore = this.store.push({
+      data: {
         id: '1',
-        type: 'book',
-      },
-      {
-        id: '2',
-        type: 'book',
+        type: 'bookstore',
+        relationships: {
+          books: {
+            data: [
+              {
+                id: '1',
+                type: 'book',
+              },
+              {
+                id: '2',
+                type: 'book',
+              },
+            ],
+          },
+        },
       },
-    ],
-  });
+      included: [
+        {
+          id: '1',
+          type: 'book',
+        },
+        {
+          id: '2',
+          type: 'book',
+        },
+      ],
+    });
 
-  assert.deepEqual(bookstore.get('books').mapBy('id'), ['1', '2'], 'initial hasmany loaded');
+    assert.deepEqual(bookstore.get('books').mapBy('id'), ['1', '2'], 'initial hasmany loaded');
 
-  let book2 = this.store.peekRecord('book', '2');
+    let book2 = this.store.peekRecord('book', '2');
 
-  await book2.destroyRecord({ adapterOptions: { clientSideDelete: true } });
+    await book2.destroyRecord({ adapterOptions: { clientSideDelete: true } });
 
-  book2.unloadRecord();
+    book2.unloadRecord();
 
-  await settled();
+    await settled();
 
-  assert.equal(this.store.hasRecordForId('book', '2'), false, 'book 2 unloaded');
-  assert.deepEqual(bookstore.get('books').mapBy('id'), ['1'], 'one book client-side deleted');
+    assert.equal(this.store.hasRecordForId('book', '2'), false, 'book 2 unloaded');
+    assert.deepEqual(bookstore.get('books').mapBy('id'), ['1'], 'one book client-side deleted');
 
-  this.store.push({
-    data: {
-      id: '2',
-      type: 'book',
-      relationships: {
-        bookstore: {
-          data: {
-            id: '1',
-            type: 'bookstore',
+    this.store.push({
+      data: {
+        id: '2',
+        type: 'book',
+        relationships: {
+          bookstore: {
+            data: {
+              id: '1',
+              type: 'bookstore',
+            },
           },
         },
       },
-    },
-  });
+    });
 
-  assert.deepEqual(
-    bookstore.get('books').mapBy('id'),
-    ['1', '2'],
-    'the deleted book (with same id) is pushed back into the store'
-  );
+    assert.deepEqual(
+      bookstore.get('books').mapBy('id'),
+      ['1', '2'],
+      'the deleted book (with same id) is pushed back into the store'
+    );
+  });
 });

tests/integration/application-test.js

@@ -131,11 +131,13 @@ module('integration/application - Attaching initializer', function(hooks) {
       },
     });
 
-    this.application = this.TestApplication.create({ autoboot: false });
+    return await run(async () => {
+      this.application = this.TestApplication.create({ autoboot: false });
 
-    await this.application.boot();
+      await this.application.boot();
 
-    assert.ok(ran, 'ember-data initializer was found');
+      assert.ok(ran, 'ember-data initializer was found');
+    });
   });
 
   test('ember-data initializer does not register the store service when it was already registered', async function(assert) {
@@ -151,14 +153,16 @@ module('integration/application - Attaching initializer', function(hooks) {
       },
     });
 
-    this.application = this.TestApplication.create({ autoboot: false });
+    return await run(async () => {
+      this.application = this.TestApplication.create({ autoboot: false });
 
-    await this.application.boot().then(() => (this.owner = this.application.buildInstance()));
+      await this.application.boot().then(() => (this.owner = this.application.buildInstance()));
 
-    let store = this.owner.lookup('service:store');
-    assert.ok(
-      store && store.get('isCustomStore'),
-      'ember-data initializer does not overwrite the previous registered service store'
-    );
+      let store = this.owner.lookup('service:store');
+      assert.ok(
+        store && store.get('isCustomStore'),
+        'ember-data initializer does not overwrite the previous registered service store'
+      );
+    });
   });
 });