chore: encapsulate InternalModel access (emberjs#6246)

packages/-ember-data/tests/unit/model-test.js

@@ -349,7 +349,7 @@ module('unit/model - Model', function(hooks) {
 
       assert.equal(person.get('id'), null, 'initial created model id should be null');
       assert.equal(idChange, 0);
-      store._setRecordId(person._internalModel, 'john');
+      person._internalModel.setId('john');
       assert.equal(idChange, 1);
       assert.equal(person.get('id'), 'john', 'new id should be correctly set.');
     });

packages/store/addon/-private/system/coerce-id.ts

@@ -6,9 +6,8 @@
 // corresponding record, we will not know if it is a string or a number.
 type Coercable = string | number | boolean | null | undefined | symbol;
 
-function coerceId(id: number | boolean | symbol): string;
-function coerceId(id: null | undefined | ''): null;
-function coerceId(id: string | undefined): string | null;
+function coerceId(id: null | undefined | string): null;
+function coerceId(id: string | number | boolean | symbol): string;
 function coerceId(id: Coercable): string | null {
   if (id === null || id === undefined || id === '') {
     return null;

packages/store/addon/-private/system/internal-model-map.ts

@@ -24,8 +24,8 @@ export default class InternalModelMap {
    * @param id {String}
    * @return {InternalModel}
    */
-  get(id: string): InternalModel | undefined {
-    return this._idToModel[id];
+  get(id: string): InternalModel | null {
+    return this._idToModel[id] || null;
   }
 
   has(id: string): boolean {
@@ -37,10 +37,7 @@ export default class InternalModelMap {
   }
 
   set(id: string, internalModel: InternalModel): void {
-    assert(
-      `You cannot index an internalModel by an empty id'`,
-      typeof id === 'string' && id.length > 0
-    );
+    assert(`You cannot index an internalModel by an empty id'`, typeof id === 'string' && id.length > 0);
     assert(
       `You cannot set an index for an internalModel to something other than an internalModel`,
       internalModel instanceof InternalModel
@@ -57,7 +54,7 @@ export default class InternalModelMap {
     this._idToModel[id] = internalModel;
   }
 
-  add(internalModel: InternalModel, id?: string): void {
+  add(internalModel: InternalModel, id: string | null): void {
     assert(
       `You cannot re-add an already present InternalModel to the InternalModelMap.`,
       !this.contains(internalModel)

packages/store/addon/-private/system/model/internal-model.ts

@@ -25,6 +25,8 @@ import { JsonApiResource, JsonApiValidationError } from '../../ts-interfaces/rec
 import { Record } from '../../ts-interfaces/record';
 import { Dict } from '../../types';
 import { RECORD_DATA_ERRORS, RECORD_DATA_STATE } from '@ember-data/canary-features';
+import { internalModelFactoryFor } from '../store/internal-model-factory';
+import coerceId from '../coerce-id';
 
 // move to TS hacks module that we can delete when this is no longer a necessary recast
 type ManyArray = InstanceType<typeof ManyArray>;
@@ -98,10 +100,6 @@ let InternalModelReferenceId = 1;
   @class InternalModel
 */
 export default class InternalModel {
-  id: string | null;
-  store: Store;
-  modelName: string;
-  clientId: string | null;
   __recordData: RecordData | null;
   _isDestroyed: boolean;
   isError: boolean;
@@ -131,12 +129,12 @@ export default class InternalModel {
   currentState: any;
   error: any;
 
-  constructor(modelName: string, id: string | null, store, data, clientId) {
-    this.id = id;
-    this.store = store;
-    this.modelName = modelName;
-    this.clientId = clientId;
-
+  constructor(
+    public modelName: string,
+    public id: string | null,
+    public store: Store,
+    public clientId?: string | null
+  ) {
     this.__recordData = null;
 
     // this ensure ordered set can quickly identify this as unique
@@ -183,7 +181,7 @@ export default class InternalModel {
 
   get _recordData(): RecordData {
     if (this.__recordData === null) {
-      let recordData = this.store._createRecordData(this.modelName, this.id, this.clientId, this);
+      let recordData = this.store._createRecordData(this.modelName, this.id, this.clientId);
       this._recordData = recordData;
       return recordData;
     }
@@ -344,7 +342,11 @@ export default class InternalModel {
         );
 
         if ('id' in properties) {
-          this.setId(properties.id);
+          const id = coerceId(properties.id);
+
+          if (id !== null) {
+            this.setId(id);
+          }
         }
 
         // convert relationship Records to RecordDatas before passing to RecordData
@@ -834,7 +836,7 @@ export default class InternalModel {
       delete this._retainedManyArrayCache[key];
     });
 
-    this.store._removeFromIdMap(this);
+    internalModelFactoryFor(this.store).remove(this);
     this._isDestroyed = true;
   }
 
@@ -997,7 +999,7 @@ export default class InternalModel {
     }
   }
 
-  notifyHasManyChange(key, record, idx) {
+  notifyHasManyChange(key: string) {
     if (this.hasRecord) {
       let manyArray = this._manyArrayCache[key];
       if (manyArray) {
@@ -1014,9 +1016,9 @@ export default class InternalModel {
     }
   }
 
-  notifyBelongsToChange(key, record) {
+  notifyBelongsToChange(key: string) {
     if (this.hasRecord) {
-      this._record.notifyBelongsToChange(key, record);
+      this._record.notifyBelongsToChange(key, this._record);
       this.updateRecordArrays();
     }
   }
@@ -1251,7 +1253,7 @@ export default class InternalModel {
     this.store.recordArrayManager.recordDidChange(this);
   }
 
-  setId(id) {
+  setId(id: string) {
     assert(
       "A record's id cannot be changed once it is in the loaded state",
       this.id === null || this.id === id || this.isNew()
@@ -1261,7 +1263,7 @@ export default class InternalModel {
     this.id = id;
 
     if (didChange && id !== null) {
-      this.store._setRecordId(this, id, this.clientId);
+      this.store.setRecordId(this.modelName, id, this.clientId as string);
     }
 
     if (didChange && this.hasRecord) {

packages/store/addon/-private/system/model/model.js

@@ -18,6 +18,7 @@ import Ember from 'ember';
 import InternalModel from './internal-model';
 import RootState from './states';
 import { RECORD_DATA_ERRORS, RECORD_DATA_STATE } from '@ember-data/canary-features';
+import coerceId from '../coerce-id';
 
 const { changeProperties } = Ember;
 
@@ -1240,7 +1241,11 @@ Object.defineProperty(Model.prototype, 'data', {
 const ID_DESCRIPTOR = {
   configurable: false,
   set(id) {
-    this._internalModel.setId(id);
+    const normalizedId = coerceId(id);
+
+    if (normalizedId !== null) {
+      this._internalModel.setId(normalizedId);
+    }
   },
 
   get() {

packages/store/addon/-private/system/record-array-manager.js

@@ -8,6 +8,7 @@ import { run as emberRunloop } from '@ember/runloop';
 import { assert } from '@ember/debug';
 import cloneNull from './clone-null';
 import { RecordArray, AdapterPopulatedRecordArray } from './record-arrays';
+import { internalModelFactoryFor } from './store/internal-model-factory';
 
 const emberRun = emberRunloop.backburner;
 
@@ -99,7 +100,7 @@ export default class RecordArrayManager {
     let pending = this._pending[modelName];
     let hasPendingChanges = Array.isArray(pending);
     let hasNoPotentialDeletions = !hasPendingChanges || pending.length === 0;
-    let map = this.store._internalModelsFor(modelName);
+    let map = internalModelFactoryFor(this.store).modelMapFor(modelName);
     let hasNoInsertionsOrRemovals = get(map, 'length') === get(array, 'length');
 
     /*
@@ -171,7 +172,7 @@ export default class RecordArrayManager {
   }
 
   _visibleInternalModelsByType(modelName) {
-    let all = this.store._internalModelsFor(modelName)._models;
+    let all = internalModelFactoryFor(this.store).modelMapFor(modelName)._models;
     let visible = [];
     for (let i = 0; i < all.length; i++) {
       let model = all[i];