fix all the things

packages/-ember-data/tests/integration/adapter/store-adapter-test.js

@@ -968,23 +968,23 @@ module('integration/adapter/store-adapter - DS.Store and DS.Adapter integration
     run(() => store.findRecord('person', 1));
   });
 
-  test('relationships returned via `commit` do not trigger additional findManys', function(assert) {
+  test('relationships returned via `commit` do not trigger additional findManys', async function(assert) {
     Person.reopen({
       dogs: DS.hasMany('dog', { async: false }),
     });
 
-    run(() => {
-      env.store.push({
-        data: {
-          type: 'dog',
-          id: '1',
-          attributes: {
-            name: 'Scruffy',
-          },
+    store.push({
+      data: {
+        type: 'dog',
+        id: '1',
+        attributes: {
+          name: 'Scruffy',
         },
-      });
+      },
     });
 
+    adapter.shouldBackgroundReloadRecord = () => false;
+
     adapter.findRecord = function(store, type, id, snapshot) {
       return resolve({
         data: {
@@ -1034,20 +1034,12 @@ module('integration/adapter/store-adapter - DS.Store and DS.Adapter integration
       assert.ok(false, 'Should not get here');
     };
 
-    return run(() => {
-      store
-        .findRecord('person', 1)
-        .then(person => {
-          return hash({ tom: person, dog: store.findRecord('dog', 1) });
-        })
-        .then(records => {
-          records.tom.get('dogs');
-          return records.dog.save();
-        })
-        .then(tom => {
-          assert.ok(true, 'Tom was saved');
-        });
-    });
+    const person = await store.findRecord('person', '1');
+    const dog = await store.findRecord('dog', '1');
+    await dog.save();
+    await person.get('dogs');
+
+    assert.ok(true, 'no findMany triggered');
   });
 
   test("relationships don't get reset if the links is the same", function(assert) {

packages/-ember-data/tests/integration/identifiers/scenarios-test.ts

@@ -11,10 +11,11 @@ import Store, {
 import Model, { attr } from '@ember-data/model';
 import Adapter from '@ember-data/adapter';
 import Serializer from '@ember-data/serializer';
-import { resolve } from 'rsvp';
+import { resolve, all } from 'rsvp';
 import { ExistingResourceObject } from '@ember-data/store/-private/ts-interfaces/ember-data-json-api';
 import { Dict } from '@ember-data/store/-private/ts-interfaces/utils';
 import { StableRecordIdentifier } from '@ember-data/store/-private/ts-interfaces/identifier';
+import { identifierCacheFor } from '@ember-data/store/-private';
 
 function isNonEmptyString(str: any): str is string {
   return typeof str === 'string' && str.length > 0;
@@ -160,6 +161,17 @@ if (IDENTIFIERS) {
         assert.strictEqual(recordById, recordByUsername, 'The records should be identical');
         assert.strictEqual(calls.findRecord, 1, 'We made one call to Adapter.findRecord');
         assert.strictEqual(calls.queryRecord, 1, 'We made one call to Adapter.queryRecord');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
       });
       test(`queryRecord with username then findRecord with id`, async function(assert) {
         const recordByUsername = await store.queryRecord('user', { username: '@runspired' });
@@ -171,6 +183,17 @@ if (IDENTIFIERS) {
         assert.strictEqual(recordById, recordByUsername, 'The records should be identical');
         assert.strictEqual(calls.findRecord, 0, 'We made zero calls to Adapter.findRecord');
         assert.strictEqual(calls.queryRecord, 1, 'We made one call to Adapter.queryRecord');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
       });
       test(`queryRecord with username and findRecord with id in parallel`, async function(assert) {
         const recordByUsernamePromise1 = store.queryRecord('user', { username: '@runspired' });
@@ -191,6 +214,17 @@ if (IDENTIFIERS) {
         assert.strictEqual(recordById, recordByUsername2, 'The records should be identical');
         assert.strictEqual(calls.findRecord, 1, 'We made one call to Adapter.findRecord');
         assert.strictEqual(calls.queryRecord, 2, 'We made two calls to Adapter.queryRecord');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername1.lid}' in ['${lids.join("', '")}']`
+        );
       });
     });
 
@@ -340,6 +374,17 @@ if (IDENTIFIERS) {
         assert.strictEqual(calls.findRecord, 1, 'We made only one call to Adapter.findRecord');
         assert.strictEqual(recordById.id, '1', 'The record id is correct');
         assert.strictEqual(identifierById.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
       });
 
       test(`findRecord by username as id then by id`, async function(assert) {
@@ -353,6 +398,121 @@ if (IDENTIFIERS) {
         assert.strictEqual(calls.findRecord, 1, 'We made one call to Adapter.findRecord');
         assert.strictEqual(recordById.id, '1', 'The record id is correct');
         assert.strictEqual(identifierById.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
+      });
+
+      test(`findRecord username and findRecord id in parallel`, async function(assert) {
+        const recordByUsernamePromise = store.findRecord('user', '@runspired');
+        const recordByIdPromise = store.findRecord('user', '1');
+
+        const [recordByUsername, recordById] = await all([recordByUsernamePromise, recordByIdPromise]);
+
+        const identifierByUsername = recordIdentifierFor(recordByUsername);
+        const identifierById = recordIdentifierFor(recordById);
+
+        assert.strictEqual(identifierById, identifierByUsername, 'The identifiers should be identical');
+        assert.strictEqual(recordById, recordByUsername, 'The records should be identical');
+        assert.strictEqual(calls.findRecord, 2, 'We made two calls to Adapter.findRecord');
+        assert.strictEqual(recordById.id, '1', 'The record id is correct');
+        assert.strictEqual(identifierById.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
+      });
+
+      test(`findRecord by username and again`, async function(assert) {
+        const recordByUsername = await store.findRecord('user', '@runspired');
+        const identifierByUsername = recordIdentifierFor(recordByUsername);
+        const recordByUsername2 = await store.findRecord('user', '@runspired');
+        const identifierByUsername2 = recordIdentifierFor(recordByUsername2);
+
+        assert.strictEqual(identifierByUsername2, identifierByUsername, 'The identifiers should be identical');
+        assert.strictEqual(recordByUsername2, recordByUsername, 'The records should be identical');
+        assert.strictEqual(calls.findRecord, 1, 'We made one call to Adapter.findRecord');
+        assert.strictEqual(recordByUsername.id, '1', 'The record id is correct');
+        assert.strictEqual(identifierByUsername.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
+      });
+
+      /*
+        Ideally in the scenario where two cache-keys refer to identical data
+        the lid cache would be pre-populated.
+
+        For `queryRecord` `store.push` and most other code-paths this will occur
+        naturally.
+
+        However prepopulation is not always possible as unfortunately `findRecord`
+        greedily creates the identifier and internalModel using what limited info
+        it has.
+
+        In these cases we have the ability to finalize to a clean state:
+
+        - no other request for the record by the other cache-key has occurred
+          => single lid, single InternalModel, single Record generated
+        
+        - another request for the record by the other cache-key occurs prior
+          to a payload having been received and used to populate any secondary
+          lid caches
+          => two lid's generated, two InternalModel's generated, single Record
+             generated. The first payload to resolve should result in a merge
+             and then
+
+        Ideally findRecord is eliminated in favor of a form of query with an
+          associated `lid`. Users may wish to implement a `findRecord` like
+          API with such behavior themselves if they encounter too many edge
+          cases with the scenario where records have multiple cache-keys in
+          the "id" position.
+      */
+      test(`findRecord by username and reload`, async function(assert) {
+        const recordByUsername = await store.findRecord('user', '@runspired');
+        const identifierByUsername = recordIdentifierFor(recordByUsername);
+        const recordByUsername2 = await store.findRecord('user', '@runspired', { reload: true });
+        const identifierByUsername2 = recordIdentifierFor(recordByUsername2);
+
+        assert.strictEqual(identifierByUsername2, identifierByUsername, 'The identifiers should be identical');
+        assert.strictEqual(recordByUsername2, recordByUsername, 'The records should be identical');
+        assert.strictEqual(calls.findRecord, 2, 'We made two calls to Adapter.findRecord');
+        assert.strictEqual(recordByUsername.id, '1', 'The record id is correct');
+        assert.strictEqual(identifierByUsername.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
       });
 
       test(`push id then findRecord username`, async function(assert) {
@@ -373,8 +533,53 @@ if (IDENTIFIERS) {
 
         assert.strictEqual(identifierById, identifierByUsername, 'The identifiers should be identical');
         assert.strictEqual(recordById, recordByUsername, 'The records should be identical');
+        assert.strictEqual(calls.findRecord, 0, 'We made zero calls to Adapter.findRecord');
         assert.strictEqual(recordById.id, '1', 'The record id is correct');
         assert.strictEqual(identifierById.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
+      });
+
+      test(`findRecord username then push id`, async function(assert) {
+        const recordByUsername = await store.findRecord('user', '@runspired');
+        const identifierByUsername = recordIdentifierFor(recordByUsername);
+        const recordById = store.push({
+          data: {
+            type: 'user',
+            id: '1',
+            attributes: {
+              username: '@runspired',
+              firstName: 'Chris',
+              age: 31,
+            },
+          },
+        });
+        const identifierById = recordIdentifierFor(recordById);
+
+        assert.strictEqual(identifierById, identifierByUsername, 'The identifiers should be identical');
+        assert.strictEqual(recordById, recordByUsername, 'The records should be identical');
+        assert.strictEqual(recordById.id, '1', 'The record id is correct');
+        assert.strictEqual(identifierById.id, '1', 'The identifier id is correct');
+
+        // ensure we truly are in a good state internally
+        const internalModels = store._internalModelsFor('user')._models;
+        assert.strictEqual(internalModels.length, 1, 'Once settled there is only a single internal-model');
+        const lidCache = identifierCacheFor(store)._cache.lids;
+        const lids = Object.keys(lidCache);
+        assert.strictEqual(
+          lids.length,
+          1,
+          `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']`
+        );
       });
     });
   });

packages/-ember-data/tests/integration/records/rematerialize-test.js

@@ -113,12 +113,20 @@ module('integration/unload - Rematerializing Unloaded Records', function(hooks)
     assert.equal(person.get('cars.length'), 1, 'The inital length of cars is correct');
 
     assert.equal(env.store.hasRecordForId('person', 1), true, 'The person is in the store');
-    assert.equal(env.store._internalModelsFor('person').has(1), true, 'The person internalModel is loaded');
+    assert.equal(
+      env.store._internalModelsFor('person').has('@ember-data:lid-person-1'),
+      true,
+      'The person internalModel is loaded'
+    );
 
     run(() => person.unloadRecord());
 
     assert.equal(env.store.hasRecordForId('person', 1), false, 'The person is unloaded');
-    assert.equal(env.store._internalModelsFor('person').has(1), false, 'The person internalModel is freed');
+    assert.equal(
+      env.store._internalModelsFor('person').has('@ember-data:lid-person-1'),
+      false,
+      'The person internalModel is freed'
+    );
 
     run(() => {
       env.store.push({
@@ -222,9 +230,17 @@ module('integration/unload - Rematerializing Unloaded Records', function(hooks)
 
     // assert our initial cache state
     assert.equal(env.store.hasRecordForId('person', '1'), true, 'The person is in the store');
-    assert.equal(env.store._internalModelsFor('person').has('1'), true, 'The person internalModel is loaded');
+    assert.equal(
+      env.store._internalModelsFor('person').has('@ember-data:lid-person-1'),
+      true,
+      'The person internalModel is loaded'
+    );
     assert.equal(env.store.hasRecordForId('boat', '1'), true, 'The boat is in the store');
-    assert.equal(env.store._internalModelsFor('boat').has('1'), true, 'The boat internalModel is loaded');
+    assert.equal(
+      env.store._internalModelsFor('boat').has('@ember-data:lid-boat-1'),
+      true,
+      'The boat internalModel is loaded'
+    );
 
     let boats = run(() => adam.get('boats'));
     assert.equal(boats.get('length'), 2, 'Before unloading boats.length is correct');
@@ -234,7 +250,11 @@ module('integration/unload - Rematerializing Unloaded Records', function(hooks)
 
     // assert our new cache state
     assert.equal(env.store.hasRecordForId('boat', '1'), false, 'The boat is unloaded');
-    assert.equal(env.store._internalModelsFor('boat').has('1'), true, 'The boat internalModel is retained');
+    assert.equal(
+      env.store._internalModelsFor('boat').has('@ember-data:lid-boat-1'),
+      true,
+      'The boat internalModel is retained'
+    );
 
     // cause a rematerialization, this should also cause us to fetch boat '1' again
     boats = run(() => adam.get('boats'));
@@ -247,6 +267,10 @@ module('integration/unload - Rematerializing Unloaded Records', function(hooks)
     assert.ok(rematerializedBoaty !== boaty, 'the boat is rematerialized, not recycled');
 
     assert.equal(env.store.hasRecordForId('boat', '1'), true, 'The boat is loaded');
-    assert.equal(env.store._internalModelsFor('boat').has('1'), true, 'The boat internalModel is retained');
+    assert.equal(
+      env.store._internalModelsFor('boat').has('@ember-data:lid-boat-1'),
+      true,
+      'The boat internalModel is retained'
+    );
   });
 });