From a37bf3e361f47cb3beeea6e36f718ad03bf2f222 Mon Sep 17 00:00:00 2001 From: Chris Thoburn Date: Sat, 16 Mar 2019 13:08:20 -0700 Subject: [PATCH] * [FEAT identifiers] adds ts interfaces for identifiers work * [FEAT identifiers] adds uuid-v4 util * [FEAT identifiers] adds ts-interface for json-api concepts w/ember-data flavor * [FEAT identifier] adds toString to interface for even more debuggability * [FEAT identifier] makes debug members of the interface optional * [FEAT IdentifierCache] implements the cache to spec * record-data-wrapper -> store-wrapper.js (fix name) * fix store usage by relationships without leaking to RecordData --- addon/-private/identifiers/cache.ts | 423 ++++++++++++++++++ addon/-private/identifiers/utils/uuid-v4.ts | 48 ++ .../internal-model/internal-model-cache.ts | 128 ++++++ addon/-private/system/coerce-id.ts | 2 +- addon/-private/system/identity-map.ts | 48 -- addon/-private/system/internal-model-map.ts | 126 ------ addon/-private/system/model/internal-model.js | 21 +- addon/-private/system/model/record-data.js | 33 +- addon/-private/system/promise-proxies.js | 3 +- addon/-private/system/record-array-manager.js | 9 +- .../-private/system/references/belongs-to.js | 8 +- .../system/relationships/state/belongs-to.js | 2 +- .../system/relationships/state/create.js | 3 +- .../system/relationships/state/has-many.js | 4 +- .../relationships/state/relationship.js | 2 +- addon/-private/system/snapshot.js | 7 +- addon/-private/system/store.js | 335 +++++--------- addon/-private/system/store/finders.js | 4 +- addon/-private/system/store/store-wrapper.js | 63 +-- addon/-private/ts-interfaces/identifier.ts | 83 ++++ addon/-private/ts-interfaces/json-api.ts | 20 + addon/-private/ts-interfaces/utils.ts | 1 + .../integration/adapter/rest-adapter-test.js | 8 +- tests/integration/records/load-test.js | 7 +- tests/integration/records/record-data-test.js | 6 +- .../integration/records/rematerialize-test.js | 22 +- tests/integration/records/unload-test.js | 77 ++-- .../relationships/belongs-to-test.js | 16 +- tests/integration/snapshot-test.js | 11 +- tests/integration/store-test.js | 3 +- tests/unit/model-test.js | 2 +- tests/unit/model/lifecycle-callbacks-test.js | 2 +- tests/unit/store/adapter-interop-test.js | 33 +- tests/unit/store/asserts-test.js | 2 - tests/unit/store/push-test.js | 2 +- tests/unit/store/unload-test.js | 2 +- 36 files changed, 1008 insertions(+), 558 deletions(-) create mode 100644 addon/-private/identifiers/cache.ts create mode 100644 addon/-private/identifiers/utils/uuid-v4.ts create mode 100644 addon/-private/internal-model/internal-model-cache.ts delete mode 100644 addon/-private/system/identity-map.ts delete mode 100644 addon/-private/system/internal-model-map.ts create mode 100644 addon/-private/ts-interfaces/identifier.ts create mode 100644 addon/-private/ts-interfaces/json-api.ts create mode 100644 addon/-private/ts-interfaces/utils.ts diff --git a/addon/-private/identifiers/cache.ts b/addon/-private/identifiers/cache.ts new file mode 100644 index 00000000000..7db87029584 --- /dev/null +++ b/addon/-private/identifiers/cache.ts @@ -0,0 +1,423 @@ +import { DEBUG } from '@glimmer/env'; +import { assert } from '@ember/debug'; +import { Dict } from '../ts-interfaces/utils'; +import { ResourceIdentifierObject, NewResourceObject } from '../ts-interfaces/json-api'; +import { + StableRecordIdentifier, + IS_IDENTIFIER, + DEBUG_CLIENT_ORIGINATED, + DEBUG_IDENTIFIER_BUCKET, + GenerationMethod, + UpdateMethod, + ForgetMethod, + ResetMethod +} from '../ts-interfaces/identifier'; +import coerceId from '../system/coerce-id'; +import uuidv4 from './utils/uuid-v4'; +import normalizeModelName from '../system/normalize-model-name'; + +type IdentifierMap = Dict; +type TypeMap = Dict; + +interface KeyOptions { + lid: IdentifierMap; + id: IdentifierMap; + _allIdentifiers: StableRecordIdentifier[]; +} + +let configuredForgetMethod: ForgetMethod | null = null; +let configuredGenerationMethod: GenerationMethod | null = null; +let configuredResetMethod: ResetMethod | null = null; +let configuredUpdateMethod: UpdateMethod | null = null; + +export function setIdentifierGenerationMethod(method: GenerationMethod): void { + configuredGenerationMethod = method; +} + +export function setIdentifierUpdateMethod(method: UpdateMethod): void { + configuredUpdateMethod = method; +} + +export function setIdentifierForgetMethod(method: ForgetMethod): void { + configuredForgetMethod = method; +} + +export function setIdentifierResetMethod(method: ResetMethod): void { + configuredResetMethod = method; +} + +function defaultGenerationMethod(data: ResourceIdentifierObject, bucket: string): string { + if (isNonEmptyString(data.lid)) { + return data.lid; + } + let { type, id } = data; + if (isNonEmptyString(id)) { + return `@ember-data:lid-${normalizeModelName(type)}-${id}`; + } + return uuidv4(); +} + +function defaultUpdateMethod(identifier: StableRecordIdentifier, data: ResourceIdentifierObject, bucket: string): void {} +function defaultForgetMethod(identifier: StableRecordIdentifier, bucket: string): void {} +function defaultResetMethod() {} + +let DEBUG_MAP; +if (DEBUG) { + DEBUG_MAP = new WeakMap(); +} + +export default class IdentifierCache { + // Typescript still leaks private properties in the final + // compiled class, so we may want to move these from _underscore + // to a WeakMap to avoid leaking + private _cache = { + lids: Object.create(null) as IdentifierMap, + types: Object.create(null) as TypeMap + }; + private _generate: GenerationMethod; + private _update: UpdateMethod; + private _forget: ForgetMethod; + private _reset: ResetMethod; + + constructor() { + // we cache the user configuredGenerationMethod at init because it must + // be configured prior and is not allowed to be changed + this._generate = configuredGenerationMethod || defaultGenerationMethod; + this._update = configuredUpdateMethod || defaultUpdateMethod; + this._forget = configuredForgetMethod || defaultForgetMethod; + this._reset = configuredResetMethod || defaultResetMethod; + } + + // allows us to peek without generating when needed + // useful for the "create" case when we need to see if + // we are accidentally overwritting something + peekRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: true): StableRecordIdentifier; + peekRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: false): StableRecordIdentifier | null; + peekRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: boolean = false): StableRecordIdentifier | null { + // short circuit if we're already the stable version + if (isStableIdentifier(resource)) { + if (DEBUG) { + // TODO should we instead just treat this case as a new generation skipping the short circuit? + if (!(resource.lid in this._cache.lids) || this._cache.lids[resource.lid] !== resource) { + throw new Error(`The supplied identifier ${resource} does not belong to this store instance`) + } + } + return resource; + } + + // `type` must always be present + if (DEBUG) { + if (!isNonEmptyString(resource.type)) { + throw new Error('resource.type needs to be a string'); + } + } + + let type = normalizeModelName(resource.type); + let keyOptions = getTypeIndex(this._cache.types, type); + let identifier: StableRecordIdentifier | null = null; + let lid = coerceId(resource.lid); + let id; + + // go straight for the stable RecordIdentifier key'd to `lid` + if (lid !== null) { + identifier = keyOptions.lid[lid] || null; + } + + // we may have not seen this resource before + // but just in case we check our own secondary lookup (`id`) + if (identifier === null) { + id = coerceId(resource.id); + + if (id !== null) { + identifier = keyOptions.id[id] || null; + } + } + + if (identifier === null) { + // we have definitely not seen this resource before + // so we allow the user configured `GenerationMethod` to tell us + let newLid = this._generate(resource, 'record'); + + // we do this _even_ when `lid` is present because secondary lookups + // may need to be populated, but we enforce not giving us something + // different than expected + if (lid !== null && newLid !== lid) { + throw new Error(`You should not change the of a RecordIdentifier`); + } else if (lid === null) { + // allow configuration to tell us that we have + // seen this `lid` before. E.g. a secondary lookup + // connects this resource to a previously seen + // resource. + identifier = keyOptions.lid[newLid] || null; + } + + if (shouldGenerate === true) { + if (identifier === null) { + // if we still don't have an identifier, time to generate one + identifier = makeStableRecordIdentifier(id, type, newLid, 'record', false); + + // populate our unique table + if (DEBUG) { + // realistically if you hit this it means you changed `type` :/ + // TODO consider how to handle type change assertions more gracefully + if (identifier.lid in this._cache.lids) { + throw new Error(`You should not change the of a RecordIdentifier`); + } + } + this._cache.lids[identifier.lid] = identifier; + + // populate our primary lookup table + // TODO consider having the `lid` cache be + // one level up + keyOptions.lid[identifier.lid] = identifier; + // TODO exists temporarily to support `peekAll` + // but likely to move + keyOptions._allIdentifiers.push(identifier); + } + + // populate our own secondary lookup table + // even for the "successful" secondary lookup + // by `_generate()`, since we missed the cache + // previously + if (id !== null) { + keyOptions.id[id] = identifier; + + // TODO allow filling out of `id` here + // for the `username` non-client created + // case. + } + } + } + + return identifier; + } + + /* + Returns the Identifier for the given Resource, creates one if it does not yet exist. + + Specifically this means that we: + + - validate the `id` `type` and `lid` combo against known identifiers + - return an object with an `lid` that is stable (repeated calls with the same + `id` + `type` or `lid` will return the same `lid` value) + - this referential stability of the object itself is guaranteed + */ + getOrCreateRecordIdentifier(resource: ResourceIdentifierObject): StableRecordIdentifier { + return this.peekRecordIdentifier(resource, true); + } + + /* + Returns a new Identifier for the supplied data. Call this method to generate + an identifier when a new resource is being created local to the client and + potentially does not have an `id`. + + Delegates generation to the user supplied `GenerateMethod` if one has been provided + with the signature `generateMethod({ type }, 'record')`. + + */ + createIdentifierForNewRecord(data: NewResourceObject): StableRecordIdentifier { + let newLid = this._generate(data, 'record'); + let identifier = makeStableRecordIdentifier(data.id || null, data.type, newLid, 'record', true); + let keyOptions = getTypeIndex(this._cache.types, data.type); + + // populate our unique table + if (DEBUG) { + if (identifier.lid in this._cache.lids) { + throw new Error(`The lid generated for the new record fails to be unique as it matches an existing identifier`); + } + } + this._cache.lids[identifier.lid] = identifier; + + // populate the type+lid cache + keyOptions.lid[newLid] = identifier; + // ensure a peekAll sees our new identifier too + // TODO move this outta here? + keyOptions._allIdentifiers.push(identifier); + + return identifier; + } + + /* + Provides the opportunity to update secondary lookup tables for existing identifiers + Called after an identifier created with `createIdentifierForNewRecord` has been + committed. + + Assigned `id` to an `Identifier` if `id` has not previously existed; however, + attempting to change the `id` or calling update without providing an `id` when + one is missing will throw an error. + + - sets `id` (if `id` was previously `null`) + - `lid` and `type` MUST NOT be altered post creation + */ + updateRecordIdentifier(identifier: StableRecordIdentifier, data: ResourceIdentifierObject): void { + if (DEBUG) { + assert( + `The supplied identifier '${identifier}' does not belong to this store instance.`, + this._cache.lids[identifier.lid] === identifier + ); + + let id = identifier.id; + let newId = coerceId(data.id); + + if (id !== null && id !== newId && newId !== null) { + let keyOptions = getTypeIndex(this._cache.types, identifier.type); + let existingIdentifier = keyOptions.id[newId]; + + if (existingIdentifier !== undefined) { + throw new Error( + `Attempted to update the 'id' for the RecordIdentifier '${identifier}' to '${newId}', but that id is already in use by '${existingIdentifier}'` + ); + } + } + } + + let id = identifier.id; + performRecordIdentifierUpdate(identifier, data, this._update); + let newId = identifier.id; + + // add to our own secondary lookup table + if (id !== newId && newId !== null) { + let keyOptions = getTypeIndex(this._cache.types, identifier.type); + keyOptions.id[newId] = identifier; + } + } + + /* + Provides the opportunity to eliminate an identifier from secondary lookup tables + as well as eliminates it from ember-data's own lookup tables and book keeping. + + Useful when a record has been deleted and the deletion has been persisted and + we do not care about the record anymore. Especially useful when an `id` of a + deleted record might be reused later for a new record. + */ + forgetRecordIdentifier(identifier: StableRecordIdentifier): void { + let keyOptions = getTypeIndex(this._cache.types, identifier.type); + if (identifier.id !== null) { + delete keyOptions.id[identifier.id]; + } + delete this._cache.lids[identifier.lid]; + delete keyOptions.lid[identifier.lid]; + + let index = keyOptions._allIdentifiers.indexOf(identifier); + keyOptions._allIdentifiers.splice(index, 1); + + this._forget(identifier, 'record'); + } +} + +function isNonEmptyString(str?: string | null): str is string { + return typeof str === 'string' && str.length > 0; +} + +function isStableIdentifier(identifier: StableRecordIdentifier | Object): identifier is StableRecordIdentifier { + return identifier[IS_IDENTIFIER] === true; +} + +function getTypeIndex(typeMap: TypeMap, type: string): KeyOptions { + let typeIndex: KeyOptions = typeMap[type]; + + if (typeIndex === undefined) { + typeIndex = { + lid: Object.create(null), + id: Object.create(null), + _allIdentifiers: [], + } as KeyOptions; + typeMap[type] = typeIndex; + } + + return typeIndex; +} + +function makeStableRecordIdentifier( + id: string | null, + type: string, + lid: string, + bucket: string, + clientOriginated: boolean = false + ): StableRecordIdentifier { + let recordIdentifier: StableRecordIdentifier = { + [IS_IDENTIFIER]: true, + lid, + id, + type + }; + + if (DEBUG) { + // we enforce immutability in dev + // but preserve our ability to do controlled updates to the reference + let wrapper: StableRecordIdentifier = { + [IS_IDENTIFIER]: true, + [DEBUG_CLIENT_ORIGINATED]: false, + [DEBUG_IDENTIFIER_BUCKET]: bucket, + get lid() { + return recordIdentifier.lid; + }, + get id() { + return recordIdentifier.id; + }, + get type() { + return recordIdentifier.type; + }, + toString() { + let { type, id, lid } = recordIdentifier; + return `${clientOriginated ? '[CLIENT_ORIGINATED] ' : ''}${type}:${id} (${lid})`; + }, + }; + Object.freeze(wrapper); + DEBUG_MAP.set(wrapper, recordIdentifier); + return wrapper; + } + + return recordIdentifier; + } + +function performRecordIdentifierUpdate( + identifier: StableRecordIdentifier, + data: ResourceIdentifierObject, + updateFn: UpdateMethod +) { + let { id, lid } = data; + let type = normalizeModelName(data.type); + + if (DEBUG) { + // get the mutable instance behind our proxy wrapper + let wrapper = identifier; + identifier = DEBUG_MAP.get(wrapper); + + if (lid !== undefined) { + let newLid = coerceId(lid); + if (newLid !== identifier.lid) { + throw new Error( + `The 'lid' for a RecordIdentifier cannot be updated once it has been created. Attempted to set lid for '${wrapper}' to '${lid}'.` + ); + } + } + + if (id !== undefined) { + let newId = coerceId(id); + + if (identifier.id !== null && identifier.id !== newId) { + throw new Error( + `The 'id' for a RecordIdentifier cannot be updated once it has been set. Attempted to set id for '${wrapper}' to '${newId}'.` + ); + } + } + + // TODO consider just ignoring here to allow flexible polymorphic support + if (type !== identifier.type) { + throw new Error( + `The 'type' for a RecordIdentifier cannot be updated once it has been set. Attempted to set type for '${wrapper}' to '${type}'.` + ); + } + + updateFn(wrapper, data, 'record'); + } else { + updateFn(identifier, data, 'record'); + } + + // upgrade the ID, this is a "one time only" ability + // TODO do we need a strong check here? + if (id !== undefined) { + identifier.id = coerceId(id); + } +} \ No newline at end of file diff --git a/addon/-private/identifiers/utils/uuid-v4.ts b/addon/-private/identifiers/utils/uuid-v4.ts new file mode 100644 index 00000000000..c2045b7e925 --- /dev/null +++ b/addon/-private/identifiers/utils/uuid-v4.ts @@ -0,0 +1,48 @@ +// support IE11 +declare global { + interface Window { + msCrypto: Crypto; + } + } + +const CRYPTO = typeof window !== 'undefined' && window.msCrypto && typeof window.msCrypto.getRandomValues === 'function' ? window.msCrypto : crypto; + +// we might be able to optimize this by requesting more bytes than we need at a time +function rng() { + // WHATWG crypto RNG - http://wiki.whatwg.org/wiki/Crypto + let rnds8 = new Uint8Array(16); + + return CRYPTO.getRandomValues(rnds8); +} + +/** + * Convert array of 16 byte values to UUID string format of the form: + * XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX + */ +const byteToHex: string[] = []; +for (let i = 0; i < 256; ++i) { + byteToHex[i] = (i + 0x100).toString(16).substr(1); +} + +function bytesToUuid(buf) { + let bth = byteToHex; + // join used to fix memory issue caused by concatenation: https://bugs.chromium.org/p/v8/issues/detail?id=3175#c4 + return ([bth[buf[0]], bth[buf[1]], + bth[buf[2]], bth[buf[3]], '-', + bth[buf[4]], bth[buf[5]], '-', + bth[buf[6]], bth[buf[7]], '-', + bth[buf[8]], bth[buf[9]], '-', + bth[buf[10]], bth[buf[11]], + bth[buf[12]], bth[buf[13]], + bth[buf[14]], bth[buf[15]]]).join(''); +} + +export default function uuidv4(): string { + let rnds = rng(); + + // Per 4.4, set bits for version and `clock_seq_hi_and_reserved` + rnds[6] = (rnds[6] & 0x0f) | 0x40; + rnds[8] = (rnds[8] & 0x3f) | 0x80; + + return bytesToUuid(rnds); +} \ No newline at end of file diff --git a/addon/-private/internal-model/internal-model-cache.ts b/addon/-private/internal-model/internal-model-cache.ts new file mode 100644 index 00000000000..9784e635b62 --- /dev/null +++ b/addon/-private/internal-model/internal-model-cache.ts @@ -0,0 +1,128 @@ +import { Dict } from '../ts-interfaces/utils'; +import { StableRecordIdentifier } from '../ts-interfaces/identifier'; +import IdentifierCache from '../identifiers/cache'; +import InternalModel from '../system/model/internal-model'; +import { assert } from '@ember/debug'; +import { NewResourceObject, ExistingResourceIdentifierObject } from '../ts-interfaces/json-api'; + +export default class InternalModelCache { + private _identifierCache: IdentifierCache; + private _types: Dict = Object.create(null); + private _lids: Dict = Object.create(null) + private _store; + + constructor(store, identifierCache) { + this._identifierCache = identifierCache; + this._store = store; + } + + all(modelName: string) { + let all = this._types[modelName] = this._types[modelName] || []; + return all; + } + + get(identifier: StableRecordIdentifier): InternalModel | null { + return this._lids[identifier.lid] || null; + } + + private _set(identifier: StableRecordIdentifier, internalModel: InternalModel): void { + this._lids[identifier.lid] = internalModel; + this.all(identifier.type).push(internalModel); + } + + createInternalModelForNewRecord(data: NewResourceObject): InternalModel { + // check for an existing identifier + let identifier; + let internalModel; + + if (data.id !== null) { + identifier = this._identifierCache.peekRecordIdentifier(data as ExistingResourceIdentifierObject, false); + internalModel = identifier !== null ? this.get(identifier) : null; + } + + if (internalModel && internalModel.hasScheduledDestroy()) { + // unloadRecord is async, if one attempts to unload + then sync create, + // we must ensure the unload is complete before starting the create + // The push path will utilize _getOrCreateInternalModelFor() + // which will call `cancelDestroy` instead for this unload + then + // sync push scenario. Once we have true client-side + // delete signaling, we should never call destroySync + internalModel.destroySync(); + internalModel = null; + this._identifierCache.forgetRecordIdentifier(identifier); + } + + assert( + `The id ${identifier.id} has already been used with another record for modelClass '${ + identifier.type + }'.`, + !internalModel + ); + + identifier = this._identifierCache.createIdentifierForNewRecord({ + type: data.type, + id: data.id + }); + + internalModel = new InternalModel(this._store, identifier); + this._set(identifier, internalModel); + + return internalModel; + } + + ensureInstance(identifier: StableRecordIdentifier): InternalModel { + let internalModel = this.get(identifier); + + if (internalModel !== null) { + // unloadRecord is async, if one attempts to unload + then sync push, + // we must ensure the unload is canceled before continuing + // The createRecord path will utilize _createInternalModel() directly + // which will call `destroySync` instead for this unload + then + // sync createRecord scenario. Once we have true client-side + // delete signaling, we should never call destroySync + if (internalModel.hasScheduledDestroy()) { + internalModel.cancelDestroy(); + } + + return internalModel; + } + + internalModel = new InternalModel(this._store, identifier); + this._set(identifier, internalModel); + + return internalModel; + } + + clear(type?: string) { + let cached: InternalModel[] = []; + + if (type !== undefined) { + let all = this.all(type); + this._types[type] = []; // clear it + cached.push(...all); + } else { + Object.keys(this._types).forEach(type => { + cached.push(...this.all(type)); + this._types[type] = []; // clear it + }); + } + + for (let i = 0; i < cached.length; i++) { + let internalModel = cached[i]; + + // this then calls "remove" + // but only once the time is right + internalModel.unloadRecord(); + } + } + + remove(identifier: StableRecordIdentifier) { + let internalModel = this._lids[identifier.lid]; + delete this._lids[identifier.lid]; + let all = this.all(identifier.type); + let index = all.indexOf(internalModel); + if (index !== -1) { + all.splice(index, 1); + } + } +} \ No newline at end of file diff --git a/addon/-private/system/coerce-id.ts b/addon/-private/system/coerce-id.ts index c45160a30ef..c1c19460765 100644 --- a/addon/-private/system/coerce-id.ts +++ b/addon/-private/system/coerce-id.ts @@ -7,7 +7,7 @@ type Coercable = string | number | boolean | null | undefined | symbol; function coerceId(id: number | boolean | symbol): string; -function coerceId(id: null | undefined | ''): null; +function coerceId(id: null | undefined | string): null; function coerceId(id: string): string | null; function coerceId(id: Coercable): string | null { if (id === null || id === undefined || id === '') { diff --git a/addon/-private/system/identity-map.ts b/addon/-private/system/identity-map.ts deleted file mode 100644 index 101d5fd6fab..00000000000 --- a/addon/-private/system/identity-map.ts +++ /dev/null @@ -1,48 +0,0 @@ -import InternalModelMap from './internal-model-map'; -import { Dict } from '../types'; - -/** - `IdentityMap` is a custom storage map for records by modelName - used by `DS.Store`. - - @class IdentityMap - @private - */ -export default class IdentityMap { - private _map: Dict = Object.create(null); - - /** - Retrieves the `InternalModelMap` for a given modelName, - creating one if one did not already exist. This is - similar to `getWithDefault` or `get` on a `MapWithDefault` - - @method retrieve - @param modelName a previously normalized modelName - @return {InternalModelMap} the InternalModelMap for the given modelName - */ - retrieve(modelName: string): InternalModelMap { - let map = this._map[modelName]; - - if (map === undefined) { - map = this._map[modelName] = new InternalModelMap(modelName); - } - - return map; - } - - /** - Clears the contents of all known `RecordMaps`, but does - not remove the InternalModelMap instances. - - @method clear - */ - clear(): void { - let map = this._map; - let keys = Object.keys(map); - - for (let i = 0; i < keys.length; i++) { - let key = keys[i]; - map[key].clear(); - } - } -} diff --git a/addon/-private/system/internal-model-map.ts b/addon/-private/system/internal-model-map.ts deleted file mode 100644 index 504b1c77866..00000000000 --- a/addon/-private/system/internal-model-map.ts +++ /dev/null @@ -1,126 +0,0 @@ -import { assert } from '@ember/debug'; -import InternalModel from './model/internal-model'; -import { Dict } from '../types'; - -/** - `InternalModelMap` is a custom storage map for internalModels of a given modelName - used by `IdentityMap`. - - It was extracted from an implicit pojo based "internalModel map" and preserves - that interface while we work towards a more official API. - - @class InternalModelMap - @private - */ -export default class InternalModelMap { - private _idToModel: Dict = Object.create(null); - private _models: InternalModel[] = []; - private _metadata: Dict | null = null; - - constructor(public modelName: string) {} - - /** - * @method get - * @param id {String} - * @return {InternalModel} - */ - get(id: string): InternalModel | undefined { - return this._idToModel[id]; - } - - has(id: string): boolean { - return !!this._idToModel[id]; - } - - get length(): number { - return this._models.length; - } - - 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 set an index for an internalModel to something other than an internalModel`, - internalModel instanceof InternalModel - ); - assert( - `You cannot set an index for an internalModel that is not in the InternalModelMap`, - this.contains(internalModel) - ); - assert( - `You cannot update the id index of an InternalModel once set. Attempted to update ${id}.`, - !this.has(id) || this.get(id) === internalModel - ); - - this._idToModel[id] = internalModel; - } - - add(internalModel: InternalModel, id?: string): void { - assert( - `You cannot re-add an already present InternalModel to the InternalModelMap.`, - !this.contains(internalModel) - ); - - if (id) { - assert( - `Duplicate InternalModel for ${this.modelName}:${id} detected.`, - !this.has(id) || this.get(id) === internalModel - ); - - this._idToModel[id] = internalModel; - } - - this._models.push(internalModel); - } - - remove(internalModel: InternalModel, id: string): void { - delete this._idToModel[id]; - - let loc = this._models.indexOf(internalModel); - - if (loc !== -1) { - this._models.splice(loc, 1); - } - } - - contains(internalModel: InternalModel): boolean { - return this._models.indexOf(internalModel) !== -1; - } - - /** - An array of all models of this modelName - @property models - @type Array - */ - get models(): InternalModel[] { - return this._models; - } - - /** - * meta information about internalModels - * @property metadata - * @type Object - */ - get metadata(): Dict { - return this._metadata || (this._metadata = Object.create(null)); - } - - /** - Destroy all models in the internalModelTest and wipe metadata. - - @method clear - */ - clear(): void { - let internalModels = this._models; - this._models = []; - - for (let i = 0; i < internalModels.length; i++) { - let internalModel = internalModels[i]; - internalModel.unloadRecord(); - } - - this._metadata = null; - } -} diff --git a/addon/-private/system/model/internal-model.js b/addon/-private/system/model/internal-model.js index 7864bba386f..c7988f18ded 100644 --- a/addon/-private/system/model/internal-model.js +++ b/addon/-private/system/model/internal-model.js @@ -84,12 +84,13 @@ let InternalModelReferenceId = 1; @class InternalModel */ export default class InternalModel { - constructor(modelName, id, store, data, clientId) { + constructor(store, identifier) { heimdall.increment(new_InternalModel); - this.id = id; + this.id = identifier.id; this.store = store; - this.modelName = modelName; - this.clientId = clientId; + this.modelName = identifier.type; + this.lid = identifier.lid; + this.identifier = identifier; this.__recordData = null; @@ -144,7 +145,7 @@ export default class InternalModel { get _recordData() { if (this.__recordData === null) { - this._recordData = this.store._createRecordData(this.modelName, this.id, this.clientId, this); + this._recordData = this.store._createRecordData(this.identifier); } return this.__recordData; } @@ -490,6 +491,7 @@ export default class InternalModel { getBelongsTo(key, options) { let resource = this._recordData.getBelongsTo(key); + let identifier = resource && resource.data ? this.store.identifierCache.getOrCreateRecordIdentifier(resource.data) : null; let relationshipMeta = this.store._relationshipMetaFor(this.modelName, null, key); let store = this.store; let parentInternalModel = this; @@ -497,8 +499,7 @@ export default class InternalModel { let isAsync = typeof async === 'undefined' ? true : async; if (isAsync) { - let internalModel = - resource && resource.data ? store._internalModelForResource(resource.data) : null; + let internalModel = identifier !== null ? store._imCache.get(identifier) : null; return PromiseBelongsTo.create({ _belongsToState: resource._relationship, promise: store._findBelongsToByJsonApiResource( @@ -510,10 +511,10 @@ export default class InternalModel { content: internalModel ? internalModel.getRecord() : null, }); } else { - if (!resource || !resource.data) { + if (identifier === null) { return null; } else { - let internalModel = store._internalModelForResource(resource.data); + let internalModel = store._imCache.ensureInstance(identifier); let toReturn = internalModel.getRecord(); assert( "You looked up the '" + @@ -1111,7 +1112,7 @@ export default class InternalModel { this.id = id; if (didChange && id !== null) { - this.store._setRecordId(this, id, this.clientId); + this.store._setRecordId(this, id, this.lid); } if (didChange && this.hasRecord) { diff --git a/addon/-private/system/model/record-data.js b/addon/-private/system/model/record-data.js index 832b7227863..0505aee6e1c 100644 --- a/addon/-private/system/model/record-data.js +++ b/addon/-private/system/model/record-data.js @@ -8,13 +8,13 @@ import coerceId from '../coerce-id'; let nextBfsId = 1; export default class RecordData { - constructor(modelName, id, clientId, storeWrapper, store) { - this.store = store; - this.modelName = modelName; + constructor(identifier, storeWrapper) { + this.modelName = identifier.type; this.__relationships = null; this.__implicitRelationships = null; - this.clientId = clientId; - this.id = id; + this.lid = identifier.lid; + this.id = identifier.id; + this.identifier = identifier; this.storeWrapper = storeWrapper; this.isDestroyed = false; this._isNew = false; @@ -27,11 +27,7 @@ export default class RecordData { // PUBLIC API getResourceIdentifier() { - return { - id: this.id, - type: this.modelName, - clientId: this.clientId, - }; + return this.identifier; } pushData(data, calculateChange) { @@ -94,7 +90,7 @@ export default class RecordData { let relationshipData = data.relationships[relationshipName]; if (DEBUG) { - let store = this.store; + let storeWrapper = this.storeWrapper; let recordData = this; let relationshipMeta = relationships[relationshipName]; if (!relationshipData || !relationshipMeta) { @@ -123,7 +119,7 @@ export default class RecordData { )}, but ${relationshipName} is a belongsTo relationship so the value must not be an array. You should probably check your data payload or serializer.`, !Array.isArray(relationshipData.data) ); - assertRelationshipData(store, recordData, relationshipData.data, relationshipMeta); + assertRelationshipData(storeWrapper, recordData, relationshipData.data, relationshipMeta); } else if (relationshipMeta.kind === 'hasMany') { assert( `A ${ @@ -136,7 +132,7 @@ export default class RecordData { if (Array.isArray(relationshipData.data)) { for (let i = 0; i < relationshipData.data.length; i++) { assertRelationshipData( - store, + storeWrapper, recordData, relationshipData.data[i], relationshipMeta @@ -232,7 +228,7 @@ export default class RecordData { } if (data.id) { // didCommit provided an ID, notify the store of it - this.storeWrapper.setRecordId(this.modelName, data.id, this.clientId); + this.storeWrapper.setRecordId(this.modelName, data.id, this.lid); this.id = coerceId(data.id); } data = data.attributes; @@ -351,11 +347,12 @@ export default class RecordData { destroy() { this._relationships.forEach((name, rel) => rel.destroy()); this.isDestroyed = true; - this.storeWrapper.disconnectRecord(this.modelName, this.id, this.clientId); + this.storeWrapper.disconnectRecord(this.modelName, this.id, this.lid); } isRecordInUse() { - return this.storeWrapper.isRecordInUse(this.modelName, this.id, this.clientId); + if (typeof this.storeWrapper.isRecordInUse !== 'function') { debugger; } + return this.storeWrapper.isRecordInUse(this.modelName, this.id, this.lid); } /** @@ -702,7 +699,7 @@ export default class RecordData { } } -function assertRelationshipData(store, recordData, data, meta) { +function assertRelationshipData(storeWrapper, recordData, data, meta) { assert( `A ${recordData.modelName} record was pushed into the store with the value of ${ meta.key @@ -735,7 +732,7 @@ function assertRelationshipData(store, recordData, data, meta) { } relationship '${meta.key}' on ${recordData}, Expected a json-api identifier with type '${ meta.type }'. No model was found for '${data.type}'.`, - data === null || !data.type || store._hasModelFor(data.type) + data === null || !data.type || storeWrapper._hasModelFor(data.type) ); } diff --git a/addon/-private/system/promise-proxies.js b/addon/-private/system/promise-proxies.js index 5307524862d..75ba0cf1fe8 100644 --- a/addon/-private/system/promise-proxies.js +++ b/addon/-private/system/promise-proxies.js @@ -106,7 +106,8 @@ export const PromiseBelongsTo = PromiseObject.extend({ let key = state.key; let store = state.store; let resource = state.recordData.getResourceIdentifier(); - let internalModel = store._internalModelForResource(resource); + let identifier = store.identifierCache.getOrCreateRecordIdentifier(resource); + let internalModel = store._imCache.get(identifier); return store.reloadBelongsTo(this, internalModel, key, options).then(() => this); }, diff --git a/addon/-private/system/record-array-manager.js b/addon/-private/system/record-array-manager.js index c1a6687db44..8724b56853f 100644 --- a/addon/-private/system/record-array-manager.js +++ b/addon/-private/system/record-array-manager.js @@ -130,8 +130,10 @@ 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 hasNoInsertionsOrRemovals = get(map, 'length') === get(array, 'length'); + + // TODO this prevents elimination of "all" behavior + let all = this.store._imCache.all(modelName) + let hasNoInsertionsOrRemovals = all.length === get(array, 'length'); /* Ideally the recordArrayManager has knowledge of the changes to be applied to @@ -204,7 +206,8 @@ export default class RecordArrayManager { } _visibleInternalModelsByType(modelName) { - let all = this.store._internalModelsFor(modelName)._models; + // TODO this prevents elimination of "all" behavior + let all = this.store._imCache.all(modelName) let visible = []; for (let i = 0; i < all.length; i++) { let model = all[i]; diff --git a/addon/-private/system/references/belongs-to.js b/addon/-private/system/references/belongs-to.js index f75d0ae4ab4..bd5ad15ee14 100644 --- a/addon/-private/system/references/belongs-to.js +++ b/addon/-private/system/references/belongs-to.js @@ -195,7 +195,8 @@ export default class BelongsToReference extends Reference { let store = this.parentInternalModel.store; let resource = this._resource(); if (resource && resource.data) { - let inverseInternalModel = store._internalModelForResource(resource.data); + let inverseIdentifier = store.identifierCache.getOrCreateRecordIdentifier(resource.data); + let inverseInternalModel = store._imCache.get(inverseIdentifier); if (inverseInternalModel && inverseInternalModel.isLoaded()) { return inverseInternalModel.getRecord(); } @@ -325,8 +326,9 @@ export default class BelongsToReference extends Reference { ); } if (resource && resource.data) { - if (resource.data && (resource.data.id || resource.data.clientId)) { - let internalModel = this.store._internalModelForResource(resource.data); + if (resource.data && (resource.data.id || resource.data.lid)) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier(resource.data); + let internalModel = this.store._imCache.ensureInstance(identifier); if (internalModel.isLoaded()) { return internalModel.reload(options).then(internalModel => { if (internalModel) { diff --git a/addon/-private/system/relationships/state/belongs-to.js b/addon/-private/system/relationships/state/belongs-to.js index 2a747da0963..87504593e2a 100644 --- a/addon/-private/system/relationships/state/belongs-to.js +++ b/addon/-private/system/relationships/state/belongs-to.js @@ -137,7 +137,7 @@ export default class BelongsToRelationship extends Relationship { storeWrapper.notifyBelongsToChange( recordData.modelName, recordData.id, - recordData.clientId, + recordData.lid, this.key ); } diff --git a/addon/-private/system/relationships/state/create.js b/addon/-private/system/relationships/state/create.js index bb0505c4fcc..21be82b4437 100644 --- a/addon/-private/system/relationships/state/create.js +++ b/addon/-private/system/relationships/state/create.js @@ -23,6 +23,7 @@ function createRelationshipFor(relationshipMeta, store, recordData, key) { export default class Relationships { constructor(recordData) { + this._store = recordData.storeWrapper.store; this.recordData = recordData; this.initializedRelationships = Object.create(null); } @@ -51,7 +52,7 @@ export default class Relationships { if (rel) { relationship = relationships[key] = createRelationshipFor( rel, - recordData.store, + this._store, recordData, key ); diff --git a/addon/-private/system/relationships/state/has-many.js b/addon/-private/system/relationships/state/has-many.js index c5c2c48caba..6107f95dfee 100755 --- a/addon/-private/system/relationships/state/has-many.js +++ b/addon/-private/system/relationships/state/has-many.js @@ -195,7 +195,7 @@ export default class ManyRelationship extends Relationship { storeWrapper.notifyPropertyChange( recordData.modelName, recordData.id, - recordData.clientId, + recordData.lid, this.key ); } @@ -206,7 +206,7 @@ export default class ManyRelationship extends Relationship { storeWrapper.notifyHasManyChange( recordData.modelName, recordData.id, - recordData.clientId, + recordData.lid, this.key ); } diff --git a/addon/-private/system/relationships/state/relationship.js b/addon/-private/system/relationships/state/relationship.js index a758c834311..53f821bd1c2 100644 --- a/addon/-private/system/relationships/state/relationship.js +++ b/addon/-private/system/relationships/state/relationship.js @@ -684,7 +684,7 @@ export default class Relationship { storeWrapper.notifyPropertyChange( recordData.modelName, recordData.id, - recordData.clientId, + recordData.lid, this.key ); } diff --git a/addon/-private/system/snapshot.js b/addon/-private/system/snapshot.js index ca36f1bcb8b..18c9131e444 100644 --- a/addon/-private/system/snapshot.js +++ b/addon/-private/system/snapshot.js @@ -246,7 +246,8 @@ export default class Snapshot { let value = relationship.getData(); let data = value && value.data; - inverseInternalModel = data && store._internalModelForResource(data); + let inverseIdentifier = data && store.identifierCache.getOrCreateRecordIdentifier(data); + inverseInternalModel = data && store._imCache.get(inverseIdentifier); if (value && value.data !== undefined) { if (inverseInternalModel && !inverseInternalModel.isDeleted()) { @@ -330,7 +331,9 @@ export default class Snapshot { if (value.data) { results = []; value.data.forEach(member => { - let internalModel = store._internalModelForResource(member); + + let identifier = store.identifierCache.getOrCreateRecordIdentifier(member); + let internalModel = store._imCache.get(identifier); if (!internalModel.isDeleted()) { if (ids) { results.push(member.id); diff --git a/addon/-private/system/store.js b/addon/-private/system/store.js index c0cc085b04b..f75bb109200 100644 --- a/addon/-private/system/store.js +++ b/addon/-private/system/store.js @@ -9,7 +9,7 @@ import { run as emberRunLoop } from '@ember/runloop'; import { set, get, computed } from '@ember/object'; import { getOwner } from '@ember/application'; import { assign } from '@ember/polyfills'; -import { default as RSVP, Promise } from 'rsvp'; +import { Promise, all, resolve, defer } from 'rsvp'; import Service from '@ember/service'; import { typeOf, isPresent, isNone } from '@ember/utils'; @@ -20,7 +20,6 @@ import { assert, deprecate, warn, inspect } from '@ember/debug'; import { DEBUG } from '@glimmer/env'; import Model from './model/model'; import normalizeModelName from './normalize-model-name'; -import IdentityMap from './identity-map'; import StoreWrapper from './store/store-wrapper'; import { promiseArray, promiseObject } from './promise-proxies'; @@ -46,6 +45,8 @@ import RecordArrayManager from './record-array-manager'; import InternalModel from './model/internal-model'; import RecordData from './model/record-data'; import edBackburner from './backburner'; +import IdentifierCache from '../identifiers/cache'; +import InternalModelCache from '../internal-model/internal-model-cache'; const badIdFormatAssertion = '`id` passed to `findRecord()` has to be non-empty string or number'; const emberRun = emberRunLoop.backburner; @@ -80,31 +81,23 @@ function promiseRecord(internalModelPromise, label) { const { _generateId, - _internalModelForId, _load, - _pushInternalModel, adapterFor, - _buildInternalModel, _didUpdateAll, normalize, peekAll, peekRecord, serializerFor, - _internalModelsFor, } = heimdall.registerMonitor( 'store', '_generateId', - '_internalModelForId', '_load', - '_pushInternalModel', 'adapterFor', - '_buildInternalModel', '_didUpdateAll', 'normalize', 'peekAll', 'peekRecord', - 'serializerFor', - '_internalModelsFor' + 'serializerFor' ); /** @@ -190,9 +183,9 @@ const Store = Service.extend({ this._backburner = edBackburner; // internal bookkeeping; not observable this.recordArrayManager = new RecordArrayManager({ store: this }); - this._identityMap = new IdentityMap(); - // To keep track of clientIds for newly created records - this._newlyCreated = new IdentityMap(); + this.identifierCache = new IdentifierCache(); + this._imCache = new InternalModelCache(this, this.identifierCache); + this._pendingSave = []; this._modelFactoryCache = Object.create(null); this._relationshipsDefCache = Object.create(null); @@ -390,8 +383,11 @@ const Store = Service.extend({ // Coerce ID to a string properties.id = coerceId(properties.id); + let internalModel = this._imCache.createInternalModelForNewRecord({ + type: normalizedModelName, + id: properties.id, + }); - let internalModel = this._buildInternalModel(normalizedModelName, properties.id); internalModel.loadedData(); // TODO this exists just to proxy `isNew` to RecordData which is weird internalModel.didCreateRecord(); @@ -754,21 +750,20 @@ const Store = Service.extend({ (typeof id === 'string' && id.length > 0) || (typeof id === 'number' && !isNaN(id)) ); - let normalizedModelName = normalizeModelName(modelName); + let type = normalizeModelName(modelName); + let normalizedId = coerceId(id); + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ type, id: normalizedId }); + let internalModel = this._imCache.ensureInstance(identifier); - let internalModel = this._internalModelForId(normalizedModelName, id); options = options || {}; - if (!this.hasRecordForId(normalizedModelName, id)) { + if (!this.hasRecordForId(type, id)) { return this._findByInternalModel(internalModel, options); } let fetchedInternalModel = this._findRecord(internalModel, options); - return promiseRecord( - fetchedInternalModel, - `DS: Store#findRecord ${normalizedModelName} with id: ${id}` - ); + return promiseRecord(fetchedInternalModel, `DS: Store#findRecord ${type} with id: ${id}`); }, _findRecord(internalModel, options) { @@ -853,7 +848,7 @@ const Store = Service.extend({ } return promiseArray( - RSVP.all(promises).then(A, null, `DS: Store#findByIds of ${normalizedModelName} complete`) + all(promises).then(A, null, `DS: Store#findByIds of ${normalizedModelName} complete`) ); }, @@ -896,7 +891,7 @@ const Store = Service.extend({ } let { id, modelName } = internalModel; - let resolver = RSVP.defer(`Fetching ${modelName}' with id: ${id}`); + let resolver = defer(`Fetching ${modelName}' with id: ${id}`); let pendingFetchItem = { internalModel, resolver, @@ -1128,9 +1123,11 @@ const Store = Service.extend({ if (DEBUG) { assertDestroyingStore(this, 'getReference'); } - let normalizedModelName = normalizeModelName(modelName); + let type = normalizeModelName(modelName); + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ type, id }); + let internalModel = this._imCache.ensureInstance(identifier); - return this._internalModelForId(normalizedModelName, id).recordReference; + return internalModel.recordReference; }, /** @@ -1170,10 +1167,12 @@ const Store = Service.extend({ `Passing classes to store methods has been removed. Please pass a dasherized string instead of ${modelName}`, typeof modelName === 'string' ); - let normalizedModelName = normalizeModelName(modelName); + let type = normalizeModelName(modelName); + let trueId = coerceId(id); + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ type, id: trueId }); - if (this.hasRecordForId(normalizedModelName, id)) { - return this._internalModelForId(normalizedModelName, id).getRecord(); + if (this.hasRecordForId(type, trueId)) { + return this._imCache.get(identifier).getRecord(); } else { return null; } @@ -1238,71 +1237,19 @@ const Store = Service.extend({ typeof modelName === 'string' ); - let normalizedModelName = normalizeModelName(modelName); - + let type = normalizeModelName(modelName); let trueId = coerceId(id); - let internalModel = this._internalModelsFor(normalizedModelName).get(trueId); + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ type, id: trueId }); + let internalModel = this._imCache.get(identifier); - return !!internalModel && internalModel.isLoaded(); - }, - - /** - Returns id record for a given type and ID. If one isn't already loaded, - it builds a new record and leaves it in the `empty` state. - - @method recordForId - @private - @param {String} modelName - @param {(String|Integer)} id - @return {DS.Model} record - */ - recordForId(modelName, id) { - if (DEBUG) { - assertDestroyingStore(this, 'recordForId'); - } - assert(`You need to pass a model name to the store's recordForId method`, isPresent(modelName)); - assert( - `Passing classes to store methods has been removed. Please pass a dasherized string instead of ${modelName}`, - typeof modelName === 'string' - ); - - return this._internalModelForId(modelName, id).getRecord(); + return internalModel !== null && internalModel.isLoaded(); }, // directly get an internal model from ID map if it is there, without doing any // processing - _getInternalModelForId(modelName, id, clientId) { - let internalModel; - if (clientId) { - internalModel = this._newlyCreatedModelsFor(modelName).get(clientId); - } - - if (!internalModel) { - internalModel = this._internalModelsFor(modelName).get(id); - } - return internalModel; - }, - - _internalModelForId(modelName, id, clientId) { - heimdall.increment(_internalModelForId); - let trueId = coerceId(id); - let internalModel = this._getInternalModelForId(modelName, trueId, clientId); - - if (internalModel) { - // unloadRecord is async, if one attempts to unload + then sync push, - // we must ensure the unload is canceled before continuing - // The createRecord path will take _existingInternalModelForId() - // which will call `destroySync` instead for this unload + then - // sync createRecord scenario. Once we have true client-side - // delete signaling, we should never call destroySync - if (internalModel.hasScheduledDestroy()) { - internalModel.cancelDestroy(); - } - - return internalModel; - } - - return this._buildInternalModel(modelName, trueId, null, clientId); + _getInternalModelForId(type, id, lid) { + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + return this._imCache.get(identifier); }, /** @@ -1364,7 +1311,7 @@ const Store = Service.extend({ _findHasManyByJsonApiResource(resource, parentInternalModel, relationshipMeta, options) { if (!resource) { - return RSVP.resolve([]); + return resolve([]); } let { @@ -1407,7 +1354,10 @@ const Store = Service.extend({ // fetch using data, pulling from local cache if possible if (!relationshipIsStale && (preferLocalCache || hasLocalPartialData)) { - let internalModels = resource.data.map(json => this._internalModelForResource(json)); + let internalModels = resource.data.map(json => { + let identifier = this.identifierCache.getOrCreateRecordIdentifier(json); + return this._imCache.get(identifier); + }); return this.findMany(internalModels, options); } @@ -1416,20 +1366,26 @@ const Store = Service.extend({ // fetch by data if (hasData || hasLocalPartialData) { - let internalModels = resource.data.map(json => this._internalModelForResource(json)); + let internalModels = resource.data.map(json => { + let identifier = this.identifierCache.getOrCreateRecordIdentifier(json); + return this._imCache.get(identifier); + }); return this._scheduleFetchMany(internalModels, options); } // we were explicitly told we have no data and no links. // TODO if the relationshipIsStale, should we hit the adapter anyway? - return RSVP.resolve([]); + return resolve([]); }, _getHasManyByJsonApiResource(resource) { let internalModels = []; if (resource && resource.data) { - internalModels = resource.data.map(reference => this._internalModelForResource(reference)); + internalModels = resource.data.map(reference => { + let identifier = this.identifierCache.getOrCreateRecordIdentifier(reference); + return this._imCache.get(identifier); + }); } return internalModels; }, @@ -1465,7 +1421,7 @@ const Store = Service.extend({ _fetchBelongsToLinkFromResource(resource, parentInternalModel, relationshipMeta, options) { if (!resource || !resource.links || !resource.links.related) { // should we warn here, not sure cause its an internal method - return RSVP.resolve(null); + return resolve(null); } return this.findBelongsTo( parentInternalModel, @@ -1485,10 +1441,13 @@ const Store = Service.extend({ _findBelongsToByJsonApiResource(resource, parentInternalModel, relationshipMeta, options) { if (!resource) { - return RSVP.resolve(null); + return resolve(null); } - let internalModel = resource.data ? this._internalModelForResource(resource.data) : null; + let identifier = resource.data + ? this.identifierCache.getOrCreateRecordIdentifier(resource.data) + : null; + let internalModel = identifier !== null ? this._imCache.get(identifier) : null; let { relationshipIsStale, allInverseRecordsAreLoaded, @@ -1533,7 +1492,7 @@ const Store = Service.extend({ We have canonical data, but our local state is empty */ if (localDataIsEmpty) { - return RSVP.resolve(null); + return resolve(null); } return this._findByInternalModel(internalModel, options); @@ -1542,7 +1501,7 @@ const Store = Service.extend({ let resourceIsLocal = !localDataIsEmpty && resource.data.id === null; if (resourceIsLocal) { - return RSVP.resolve(internalModel.getRecord()); + return resolve(internalModel.getRecord()); } // fetch by data @@ -1554,7 +1513,7 @@ const Store = Service.extend({ // we were explicitly told we have no data and no links. // TODO if the relationshipIsStale, should we hit the adapter anyway? - return RSVP.resolve(null); + return resolve(null); }, /** @@ -2019,7 +1978,6 @@ const Store = Service.extend({ */ _fetchAll(modelName, array, options = {}) { let adapter = this.adapterFor(modelName); - let sinceToken = this._internalModelsFor(modelName).metadata.since; assert(`You tried to load all records but you have no adapter (for ${modelName})`, adapter); assert( @@ -2029,14 +1987,14 @@ const Store = Service.extend({ if (options.reload) { set(array, 'isUpdating', true); - return promiseArray(_findAll(adapter, this, modelName, sinceToken, options)); + return promiseArray(_findAll(adapter, this, modelName, options)); } let snapshotArray = array._createSnapshot(options); if (adapter.shouldReloadAll(this, snapshotArray)) { set(array, 'isUpdating', true); - return promiseArray(_findAll(adapter, this, modelName, sinceToken, options)); + return promiseArray(_findAll(adapter, this, modelName, options)); } if (options.backgroundReload === false) { @@ -2045,7 +2003,7 @@ const Store = Service.extend({ if (options.backgroundReload || adapter.shouldBackgroundReloadAll(this, snapshotArray)) { set(array, 'isUpdating', true); - _findAll(adapter, this, modelName, sinceToken, options); + _findAll(adapter, this, modelName, options); } return promiseArray(Promise.resolve(array)); @@ -2123,20 +2081,13 @@ const Store = Service.extend({ ); if (arguments.length === 0) { - this._identityMap.clear(); + this._imCache.clear(); } else { let normalizedModelName = normalizeModelName(modelName); - this._internalModelsFor(normalizedModelName).clear(); + this._imCache.clear(normalizedModelName); } }, - filter() { - assert( - 'The filter API has been moved to a plugin. To enable store.filter using an environment flag, or to use an alternative, you can visit the ember-data-filter addon page. https://github.com/ember-data/ember-data-filter', - false - ); - }, - // .............. // . PERSISTING . // .............. @@ -2274,15 +2225,15 @@ const Store = Service.extend({ @private @param {String} modelName @param {string} newId - @param {number} clientId + @param {number} lid */ - setRecordId(modelName, newId, clientId) { + setRecordId(modelName, newId, lid) { let trueId = coerceId(newId); - let internalModel = this._getInternalModelForId(modelName, trueId, clientId); - this._setRecordId(internalModel, newId, clientId); + let internalModel = this._getInternalModelForId(modelName, trueId, lid); + this._setRecordId(internalModel, newId, lid); }, - _setRecordId(internalModel, id, clientId) { + _setRecordId(internalModel, id, lid) { if (DEBUG) { assertDestroyingStore(this, 'setRecordId'); } @@ -2311,32 +2262,16 @@ const Store = Service.extend({ return; } - let existingInternalModel = this._existingInternalModelForId(modelName, id); - - assert( - `'${modelName}' was saved to the server, but the response returned the new id '${id}', which has already been used with another record.'`, - isNone(existingInternalModel) || existingInternalModel === internalModel - ); + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ + type: modelName, + lid: coerceId(lid), + }); - this._internalModelsFor(internalModel.modelName).set(id, internalModel); - this._newlyCreatedModelsFor(internalModel.modelName).remove(internalModel, clientId); + this.identifierCache.updateRecordIdentifier(identifier, { type: modelName, id }); internalModel.setId(id); }, - /** - Returns a map of IDs to client IDs for a given modelName. - - @method _internalModelsFor - @private - @param {String} modelName - @return {Object} recordMap - */ - _internalModelsFor(modelName) { - heimdall.increment(_internalModelsFor); - return this._identityMap.retrieve(modelName); - }, - _newlyCreatedModelsFor(modelName) { return this._newlyCreated.retrieve(modelName); }, @@ -2354,8 +2289,8 @@ const Store = Service.extend({ */ _load(data) { heimdall.increment(_load); - let modelName = normalizeModelName(data.type); - let internalModel = this._internalModelForId(modelName, data.id); + let identifier = this.identifierCache.getOrCreateRecordIdentifier(data); + let internalModel = this._imCache.ensureInstance(identifier); let isUpdate = internalModel.currentState.isEmpty === false; @@ -2367,6 +2302,8 @@ const Store = Service.extend({ this.recordArrayManager.recordWasLoaded(internalModel); } + this.identifierCache.updateRecordIdentifier(identifier, data); + return internalModel; }, @@ -2639,7 +2576,7 @@ const Store = Service.extend({ if (included) { for (i = 0, length = included.length; i < length; i++) { - this._pushInternalModel(included[i]); + this._pushResource(included[i]); } } @@ -2648,7 +2585,7 @@ const Store = Service.extend({ let internalModels = new Array(length); for (i = 0; i < length; i++) { - internalModels[i] = this._pushInternalModel(jsonApiDoc.data[i]); + internalModels[i] = this._pushResource(jsonApiDoc.data[i]); } return internalModels; } @@ -2664,15 +2601,14 @@ const Store = Service.extend({ typeOf(jsonApiDoc.data) === 'object' ); - return this._pushInternalModel(jsonApiDoc.data); + return this._pushResource(jsonApiDoc.data); }); heimdall.stop(token); return internalModelOrModels; }, - _pushInternalModel(data) { - heimdall.increment(_pushInternalModel); - let modelName = data.type; + _pushResource(data) { + let modelName = normalizeModelName(data.type); assert( `You must include an 'id' for ${modelName} in an object passed to 'push'`, data.id !== null && data.id !== undefined && data.id !== '' @@ -2839,34 +2775,34 @@ const Store = Service.extend({ return relationships; }, - _internalModelForResource(resource) { - let internalModel; - if (resource.clientId) { - internalModel = this._newlyCreatedModelsFor(resource.type).get(resource.clientId); - } - if (!internalModel) { - internalModel = this._internalModelForId(resource.type, resource.id); - } - return internalModel; + _createRecordData(identifier) { + return this.createRecordDataFor( + identifier.type, + identifier.id, + identifier.lid, + this.storeWrapper + ); }, - _createRecordData(modelName, id, clientId, internalModel) { - return this.createRecordDataFor(modelName, id, clientId, this.storeWrapper); + createRecordDataFor(type, id, lid, storeWrapper) { + let identifier = this.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + return new RecordData(identifier, storeWrapper, this); }, - createRecordDataFor(modelName, id, clientId, storeWrapper) { - return new RecordData(modelName, id, clientId, storeWrapper, this); - }, + recordDataFor(identifier) { + let internalModel = this._imCache.ensureInstance(identifier); - recordDataFor(modelName, id, clientId) { - let internalModel = this._internalModelForId(modelName, id, clientId); return recordDataFor(internalModel); }, + // TODO this meanas that hasMany needs to not use this once singleton lands + // so a minimal refactor for using identifiers in relationships must occur in some form _internalModelForRecordData(recordData) { let resource = recordData.getResourceIdentifier(); - return this._internalModelForId(resource.type, resource.id, resource.clientId); + let identifier = this.identifierCache.getOrCreateRecordIdentifier(resource); + return this._imCache.ensureInstance(identifier); }, + /** `normalize` converts a json payload into the normalized form that [push](#method_push) expects. @@ -2907,62 +2843,6 @@ const Store = Service.extend({ newClientId() { return globalClientIdCounter++; }, - /** - Build a brand new record for a given type, ID, and - initial data. - - @method _buildInternalModel - @private - @param {String} modelName - @param {String} id - @param {Object} data - @return {InternalModel} internal model - */ - _buildInternalModel(modelName, id, data, clientId) { - heimdall.increment(_buildInternalModel); - - assert( - `You can no longer pass a modelClass as the first argument to store._buildInternalModel. Pass modelName instead.`, - typeof modelName === 'string' - ); - - let existingInternalModel = this._existingInternalModelForId(modelName, id); - - assert( - `The id ${id} has already been used with another record for modelClass '${modelName}'.`, - !existingInternalModel - ); - - if (id === null && !clientId) { - clientId = this.newClientId(); - } - // lookupFactory should really return an object that creates - // instances with the injections applied - let internalModel = new InternalModel(modelName, id, this, data, clientId); - if (clientId) { - this._newlyCreatedModelsFor(modelName).add(internalModel, clientId); - } - - this._internalModelsFor(modelName).add(internalModel, id); - - return internalModel; - }, - - _existingInternalModelForId(modelName, id) { - let internalModel = this._internalModelsFor(modelName).get(id); - - if (internalModel && internalModel.hasScheduledDestroy()) { - // unloadRecord is async, if one attempts to unload + then sync create, - // we must ensure the unload is complete before starting the create - // The push path will take _internalModelForId() - // which will call `cancelDestroy` instead for this unload + then - // sync push scenario. Once we have true client-side - // delete signaling, we should never call destroySync - internalModel.destroySync(); - internalModel = null; - } - return internalModel; - }, //Called by the state machine to notify the store that the record is ready to be interacted with recordWasLoaded(record) { @@ -2985,11 +2865,11 @@ const Store = Service.extend({ @param {InternalModel} internalModel */ _removeFromIdMap(internalModel) { - let recordMap = this._internalModelsFor(internalModel.modelName); - let id = internalModel.id; + let identifier = internalModel.identifier; + this._imCache.remove(identifier); - recordMap.remove(internalModel, id); - //TODO IGOR DAVID remove from client id map + // allow this identifier to be reused by a different record + this.identifierCache.forgetRecordIdentifier(identifier); }, // ...................... @@ -3258,7 +3138,8 @@ const Store = Service.extend({ ); //TODO:Better asserts - return this._internalModelForId(resourceIdentifier.type, resourceIdentifier.id); + let identifier = this.identifierCache.getOrCreateRecordIdentifier(resourceIdentifier); + return this._imCache.ensureInstance(identifier); }, _pushResourceIdentifiers(relationship, resourceIdentifiers) { @@ -3277,11 +3158,11 @@ const Store = Service.extend({ Array.isArray(resourceIdentifiers) ); - let _internalModels = new Array(resourceIdentifiers.length); + let internalModels = new Array(resourceIdentifiers.length); for (let i = 0; i < resourceIdentifiers.length; i++) { - _internalModels[i] = this._pushResourceIdentifier(relationship, resourceIdentifiers[i]); + internalModels[i] = this._pushResourceIdentifier(relationship, resourceIdentifiers[i]); } - return _internalModels; + return internalModels; }, }); diff --git a/addon/-private/system/store/finders.js b/addon/-private/system/store/finders.js index f891d7380ea..e3430430b1a 100644 --- a/addon/-private/system/store/finders.js +++ b/addon/-private/system/store/finders.js @@ -366,12 +366,12 @@ export function _findBelongsTo(adapter, store, internalModel, link, relationship ); } -export function _findAll(adapter, store, modelName, sinceToken, options) { +export function _findAll(adapter, store, modelName, options) { let modelClass = store.modelFor(modelName); // adapter.findAll depends on the class let recordArray = store.peekAll(modelName); let snapshotArray = recordArray._createSnapshot(options); let promise = Promise.resolve().then(() => - adapter.findAll(store, modelClass, sinceToken, snapshotArray) + adapter.findAll(store, modelClass, undefined, snapshotArray) ); let label = 'DS: Handle Adapter#findAll of ' + modelClass; diff --git a/addon/-private/system/store/store-wrapper.js b/addon/-private/system/store/store-wrapper.js index d309cd2f275..26a8a027b02 100644 --- a/addon/-private/system/store/store-wrapper.js +++ b/addon/-private/system/store/store-wrapper.js @@ -5,9 +5,16 @@ export default class StoreWrapper { this._pendingManyArrayUpdates = null; } - _scheduleManyArrayUpdate(modelName, id, clientId, key) { + // TODO this exists just to the default recordData can + // check this in debug for relationships + // we can likely do away with this in the new relationship layer + _hasModelFor(modelName) { + return this.store._hasModelFor(modelName); + } + + _scheduleManyArrayUpdate(identifier, key) { let pending = (this._pendingManyArrayUpdates = this._pendingManyArrayUpdates || []); - pending.push(modelName, id, clientId, key); + pending.push(identifier, key); if (this._willUpdateManyArrays === true) { return; @@ -31,13 +38,11 @@ export default class StoreWrapper { this._willUpdateManyArrays = false; let store = this.store; - for (let i = 0; i < pending.length; i += 4) { - let modelName = pending[i]; - let id = pending[i + 1]; - let clientId = pending[i + 2]; - let key = pending[i + 3]; - let internalModel = store._getInternalModelForId(modelName, id, clientId); - internalModel.notifyHasManyChange(key); + for (let i = 0; i < pending.length; i += 2) { + let identifier = pending[i]; + let key = pending[i + 1]; + let internalModel = store._imCache.get(identifier); + internalModel && internalModel.notifyHasManyChange(key); } } @@ -60,38 +65,46 @@ export default class StoreWrapper { return this.relationshipsDefinitionFor(modelName)[key]._inverseIsAsync(this.store, modelClass); } - notifyPropertyChange(modelName, id, clientId, key) { - let internalModel = this.store._getInternalModelForId(modelName, id, clientId); - internalModel.notifyPropertyChange(key); + notifyPropertyChange(type, id, lid, key) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + let internalModel = this.store._imCache.get(identifier); + internalModel && internalModel.notifyPropertyChange(key); } - notifyHasManyChange(modelName, id, clientId, key) { - this._scheduleManyArrayUpdate(modelName, id, clientId, key); + notifyHasManyChange(type, id, lid, key) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + this._scheduleManyArrayUpdate(identifier, key); } - notifyBelongsToChange(modelName, id, clientId, key) { - let internalModel = this.store._getInternalModelForId(modelName, id, clientId); - internalModel.notifyBelongsToChange(key); + notifyBelongsToChange(type, id, lid, key) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + let internalModel = this.store._imCache.get(identifier); + internalModel && internalModel.notifyBelongsToChange(key); } - recordDataFor(modelName, id, clientId) { - return this.store.recordDataFor(modelName, id, clientId); + recordDataFor(type, id, lid) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + return this.store.recordDataFor(identifier); } - setRecordId(modelName, id, clientId) { - this.store.setRecordId(modelName, id, clientId); + setRecordId(type, id, lid) { + this.store.setRecordId(type, id, lid); } - isRecordInUse(modelName, id, clientId) { - let internalModel = this.store._getInternalModelForId(modelName, id, clientId); + isRecordInUse(type, id, lid) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + let internalModel = this.store._imCache.get(identifier); + if (!internalModel) { return false; } return internalModel.isRecordInUse(); } - disconnectRecord(modelName, id, clientId) { - let internalModel = this.store._getInternalModelForId(modelName, id, clientId); + disconnectRecord(type, id, lid) { + let identifier = this.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + let internalModel = this.store._imCache.get(identifier); + if (internalModel) { internalModel.destroyFromRecordData(); } diff --git a/addon/-private/ts-interfaces/identifier.ts b/addon/-private/ts-interfaces/identifier.ts new file mode 100644 index 00000000000..c9b1a1b1e4f --- /dev/null +++ b/addon/-private/ts-interfaces/identifier.ts @@ -0,0 +1,83 @@ +export const IS_IDENTIFIER = Symbol('is-identifier'); + +// provided for additional debuggability +export const DEBUG_CLIENT_ORIGINATED = Symbol('record-originated-on-client'); +export const DEBUG_IDENTIFIER_BUCKET = Symbol('identifier-bucket'); + +export interface Identifier { + lid: string; +} + +export interface StableIdentifier extends Identifier { + [IS_IDENTIFIER]: true; + [DEBUG_IDENTIFIER_BUCKET]?: string; + toString?(): string; +} + +export interface RecordIdentifier extends Identifier { + id: string | null; + type: string; +} + +export interface StableRecordIdentifier extends StableIdentifier { + id: string | null; + type: string; + [DEBUG_CLIENT_ORIGINATED]?: boolean; +} + +/* + A method which can expect to receive various data as its first argument + and the name of a bucket as its second argument. Currently the second + argument will always be `record` data should conform to a `json-api` + `Resource` interface, but will be the normalized json data for a single + resource that has been given to the store. + + The method must return a unique (to at-least the given bucket) string identifier + for the given data as a string to be used as the `lid` of an `Identifier` token. + + This method will only be called by either `getOrCreateIdentifier` or + `createIdentifierForNewRecord` when an identifier for the supplied data + is not already known via `lid` or `type + id` combo and one needs to be + generated or retrieved from a proprietary cache. + + `data` will be the same data argument provided to `getOrCreateIdentifier` + and in the `createIdentifierForNewRecord` case will be an object with + only `type` as a key. +*/ +export type GenerationMethod = (data: Object, bucket: string) => string; + +/* + A method which can expect to receive an existing `Identifier` alongside + some new data to consider as a second argument. This is an opportunity + for secondary lookup tables and caches associated with the identifier + to be amended. + + This method is called everytime `updateRecordIdentifier` is called and + with the same arguments. It provides the opportunity to update secondary + lookup tables for existing identifiers. + + It will always be called after an identifier created with `createIdentifierForNewRecord` + has been committed, or after an update to the `record` a `RecordIdentifier` + is assigned to has been committed. Committed here meaning that the server + has acknowledged the update (for instance after a call to `.save()`) + + If `id` has not previously existed, it will be assigned to the `Identifier` + prior to this `UpdateMethod` being called; however, calls to the parent method + `updateRecordIdentifier` that attempt to change the `id` or calling update + without providing an `id` when one is missing will throw an error. +*/ +export type UpdateMethod = (identifier: StableIdentifier, newData: Object, bucket: string) => void; + +/* +A method which can expect to receive an existing `Identifier` that should be eliminated + from any secondary lookup tables or caches that the user has populated for it. +*/ +export type ForgetMethod = (identifier: StableIdentifier, bucket: string) => void; + +/* + A method which can expect to be called when the parent application is destroyed. + + If you have properly used a WeakMap to encapsulate the state of your customization + to the application instance, you may not need to implement the `resetMethod`. +*/ +export type ResetMethod = () => void; \ No newline at end of file diff --git a/addon/-private/ts-interfaces/json-api.ts b/addon/-private/ts-interfaces/json-api.ts new file mode 100644 index 00000000000..d70bc1bd503 --- /dev/null +++ b/addon/-private/ts-interfaces/json-api.ts @@ -0,0 +1,20 @@ +import string from "ember-data/transforms/string"; + +export interface ExistingResourceIdentifierObject { + lid?: string; + id: string; + type: string; +} + +export interface NewResourceIdentifierObject { + lid: string; + id: string | null; + type: string; +} + +export type ResourceIdentifierObject = ExistingResourceIdentifierObject | NewResourceIdentifierObject; + +export interface NewResourceObject { + id: string | null; + type: string; +} \ No newline at end of file diff --git a/addon/-private/ts-interfaces/utils.ts b/addon/-private/ts-interfaces/utils.ts new file mode 100644 index 00000000000..e6126281365 --- /dev/null +++ b/addon/-private/ts-interfaces/utils.ts @@ -0,0 +1 @@ +export type Dict = { [KK in K]: V }; \ No newline at end of file diff --git a/tests/integration/adapter/rest-adapter-test.js b/tests/integration/adapter/rest-adapter-test.js index 82e8a45c23b..702c736b605 100644 --- a/tests/integration/adapter/rest-adapter-test.js +++ b/tests/integration/adapter/rest-adapter-test.js @@ -12,6 +12,10 @@ import Pretender from 'pretender'; import DS from 'ember-data'; +function internalModelsFor(store, modelName) { + return store._imCache.all(modelName); +} + let env, store, adapter, Post, Comment, SuperUser; let passedUrl, passedVerb, passedHash; let server; @@ -626,12 +630,12 @@ test("createRecord - response can contain relationships the client doesn't yet k 'the comments are related to the correct post model' ); assert.equal( - store._internalModelsFor('post').models.length, + internalModelsFor(store, 'post').length, 1, 'There should only be one post record in the store' ); - let postRecords = store._internalModelsFor('post').models; + let postRecords = internalModelsFor(store, 'post'); for (var i = 0; i < postRecords.length; i++) { assert.equal( post, diff --git a/tests/integration/records/load-test.js b/tests/integration/records/load-test.js index 95aa84775a4..8fd1dd18732 100644 --- a/tests/integration/records/load-test.js +++ b/tests/integration/records/load-test.js @@ -9,6 +9,11 @@ import { attr, belongsTo } from '@ember-decorators/data'; import { run } from '@ember/runloop'; import todo from '../../helpers/todo'; +function internalModelFor(store, type, id) { + let identifier = store.identifierCache.getOrCreateRecordIdentifier({ type, id }); + return store._imCache.ensureInstance(identifier); +} + class Person extends Model { @attr name; @@ -120,7 +125,7 @@ module('integration/load - Loading Records', function(hooks) { }) ); - let internalModel = store._internalModelForId('person', '1'); + let internalModel = internalModelFor(store, 'person', '1'); // test that our initial state is correct assert.equal(internalModel.isEmpty(), true, 'We begin in the empty state'); diff --git a/tests/integration/records/record-data-test.js b/tests/integration/records/record-data-test.js index 780db925daa..ffefdf8ba77 100644 --- a/tests/integration/records/record-data-test.js +++ b/tests/integration/records/record-data-test.js @@ -33,10 +33,10 @@ module('RecordData Compatibility', function(hooks) { }); class CustomRecordData { - constructor(modelName, id, clientId, storeWrapper) { + constructor(modelName, id, lid, storeWrapper) { this.type = modelName; this.id = id || null; - this.clientId = clientId; + this.lid = lid; this.storeWrapper = storeWrapper; this.attributes = null; this.relationships = null; @@ -72,7 +72,7 @@ module('RecordData Compatibility', function(hooks) { return { id: this.id, type: this.type, - clientId: this.clientId, + lid: this.lid, }; } // TODO missing from RFC but required to implement diff --git a/tests/integration/records/rematerialize-test.js b/tests/integration/records/rematerialize-test.js index c55e59c4c94..a7cf214898d 100644 --- a/tests/integration/records/rematerialize-test.js +++ b/tests/integration/records/rematerialize-test.js @@ -8,6 +8,16 @@ import DS from 'ember-data'; const { attr, belongsTo, hasMany, Model } = DS; +function internalModelsFor(store, modelName) { + return store._imCache.all(modelName); +} +function hasInternalModelFor(store, type, id) { + let internalModels = internalModelsFor(store, type); + let filtered = internalModels.filter(v => v.id === id); + + return filtered.length === 1; +} + let env; let Person = Model.extend({ @@ -115,7 +125,7 @@ test('a sync belongs to relationship to an unloaded record can restore that reco assert.equal(env.store.hasRecordForId('person', 1), true, 'The person is in the store'); assert.equal( - env.store._internalModelsFor('person').has(1), + hasInternalModelFor(env.store, 'person', '1'), true, 'The person internalModel is loaded' ); @@ -124,7 +134,7 @@ test('a sync belongs to relationship to an unloaded record can restore that reco assert.equal(env.store.hasRecordForId('person', 1), false, 'The person is unloaded'); assert.equal( - env.store._internalModelsFor('person').has(1), + hasInternalModelFor(env.store, 'person', '1'), false, 'The person internalModel is freed' ); @@ -232,13 +242,13 @@ test('an async has many relationship to an unloaded record can restore that reco // 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'), + hasInternalModelFor(env.store, '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'), + hasInternalModelFor(env.store, 'boat', '1'), true, 'The boat internalModel is loaded' ); @@ -252,7 +262,7 @@ test('an async has many relationship to an unloaded record can restore that reco // 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'), + hasInternalModelFor(env.store, 'boat', '1'), true, 'The boat internalModel is retained' ); @@ -273,7 +283,7 @@ test('an async has many relationship to an unloaded record can restore that reco assert.equal(env.store.hasRecordForId('boat', '1'), true, 'The boat is loaded'); assert.equal( - env.store._internalModelsFor('boat').has('1'), + hasInternalModelFor(env.store, 'boat', '1'), true, 'The boat internalModel is retained' ); diff --git a/tests/integration/records/unload-test.js b/tests/integration/records/unload-test.js index 0b5f1d5bcd1..43c117694e5 100644 --- a/tests/integration/records/unload-test.js +++ b/tests/integration/records/unload-test.js @@ -12,6 +12,10 @@ function idsFromOrderedSet(set) { return set.list.map(i => i.id); } +function internalModelsFor(store, modelName) { + return store._imCache.all(modelName); +} + const { attr, belongsTo, hasMany, Model } = DS; const Person = Model.extend({ @@ -177,14 +181,14 @@ module('integration/unload - Unloading Records', function(hooks) { }); assert.equal(store.peekAll('person').get('length'), 1, 'one person record loaded'); - assert.equal(store._internalModelsFor('person').length, 1, 'one person internalModel loaded'); + assert.equal(internalModelsFor(store, 'person').length, 1, 'one person internalModel loaded'); run(function() { adam.unloadRecord(); }); assert.equal(store.peekAll('person').get('length'), 0, 'no person records'); - assert.equal(store._internalModelsFor('person').length, 0, 'no person internalModels'); + assert.equal(internalModelsFor(store, 'person').length, 0, 'no person internalModels'); }); test('can unload all records for a given type', function(assert) { @@ -232,9 +236,9 @@ module('integration/unload - Unloading Records', function(hooks) { }); assert.equal(store.peekAll('person').get('length'), 2, 'two person records loaded'); - assert.equal(store._internalModelsFor('person').length, 2, 'two person internalModels loaded'); + assert.equal(internalModelsFor(store, 'person').length, 2, 'two person internalModels loaded'); assert.equal(store.peekAll('car').get('length'), 1, 'one car record loaded'); - assert.equal(store._internalModelsFor('car').length, 1, 'one car internalModel loaded'); + assert.equal(internalModelsFor(store, 'car').length, 1, 'one car internalModel loaded'); run(function() { car.get('person'); @@ -243,8 +247,8 @@ module('integration/unload - Unloading Records', function(hooks) { assert.equal(store.peekAll('person').get('length'), 0); assert.equal(store.peekAll('car').get('length'), 1); - assert.equal(store._internalModelsFor('person').length, 0, 'zero person internalModels loaded'); - assert.equal(store._internalModelsFor('car').length, 1, 'one car internalModel loaded'); + assert.equal(internalModelsFor(store, 'person').length, 0, 'zero person internalModels loaded'); + assert.equal(internalModelsFor(store, 'car').length, 1, 'one car internalModel loaded'); run(function() { store.push({ @@ -323,9 +327,9 @@ module('integration/unload - Unloading Records', function(hooks) { }); assert.equal(store.peekAll('person').get('length'), 2, 'two person records loaded'); - assert.equal(store._internalModelsFor('person').length, 2, 'two person internalModels loaded'); + assert.equal(internalModelsFor(store, 'person').length, 2, 'two person internalModels loaded'); assert.equal(store.peekAll('car').get('length'), 1, 'one car record loaded'); - assert.equal(store._internalModelsFor('car').length, 1, 'one car internalModel loaded'); + assert.equal(internalModelsFor(store, 'car').length, 1, 'one car internalModel loaded'); run(function() { store.unloadAll(); @@ -333,8 +337,8 @@ module('integration/unload - Unloading Records', function(hooks) { assert.equal(store.peekAll('person').get('length'), 0); assert.equal(store.peekAll('car').get('length'), 0); - assert.equal(store._internalModelsFor('person').length, 0, 'zero person internalModels loaded'); - assert.equal(store._internalModelsFor('car').length, 0, 'zero car internalModels loaded'); + assert.equal(internalModelsFor(store, 'person').length, 0, 'zero person internalModels loaded'); + assert.equal(internalModelsFor(store, 'car').length, 0, 'zero car internalModels loaded'); }); test('removes findAllCache after unloading all records', function(assert) { @@ -365,7 +369,7 @@ module('integration/unload - Unloading Records', function(hooks) { }); assert.equal(store.peekAll('person').get('length'), 2, 'two person records loaded'); - assert.equal(store._internalModelsFor('person').length, 2, 'two person internalModels loaded'); + assert.equal(internalModelsFor(store, 'person').length, 2, 'two person internalModels loaded'); run(function() { store.peekAll('person'); @@ -373,7 +377,7 @@ module('integration/unload - Unloading Records', function(hooks) { }); assert.equal(store.peekAll('person').get('length'), 0, 'zero person records loaded'); - assert.equal(store._internalModelsFor('person').length, 0, 'zero person internalModels loaded'); + assert.equal(internalModelsFor(store, 'person').length, 0, 'zero person internalModels loaded'); }); test('unloading all records also updates record array from peekAll()', function(assert) { @@ -449,12 +453,12 @@ module('integration/unload - Unloading Records', function(hooks) { let boat = store.peekRecord('boat', '1'); let initialBoatInternalModel = boat._internalModel; let relationshipState = person.hasMany('boats').hasManyRelationship; - let knownPeople = store._internalModelsFor('person'); - let knownBoats = store._internalModelsFor('boat'); + let knownPeople = internalModelsFor(store, 'person'); + let knownBoats = internalModelsFor(store, 'boat'); // ensure we loaded the people and boats - assert.equal(knownPeople.models.length, 1, 'one person record is loaded'); - assert.equal(knownBoats.models.length, 1, 'one boat record is loaded'); + assert.equal(knownPeople.length, 1, 'one person record is loaded'); + assert.equal(knownBoats.length, 1, 'one boat record is loaded'); assert.equal(store.hasRecordForId('person', '1'), true); assert.equal(store.hasRecordForId('boat', '1'), true); @@ -473,8 +477,8 @@ module('integration/unload - Unloading Records', function(hooks) { }); // ensure that our new state is correct - assert.equal(knownPeople.models.length, 1, 'one person record is loaded'); - assert.equal(knownBoats.models.length, 0, 'no boat records are loaded'); + assert.equal(knownPeople.length, 1, 'one person record is loaded'); + assert.equal(knownBoats.length, 1, 'we still have a boat'); assert.equal( relationshipState.canonicalMembers.size, 1, @@ -499,7 +503,7 @@ module('integration/unload - Unloading Records', function(hooks) { }); test('unloadAll(type) does not leave stranded internalModels in relationships (rediscover via relationship reload)', function(assert) { - assert.expect(17); + assert.expect(18); adapter.findRecord = (store, type, id) => { assert.ok(type.modelName === 'boat', 'We refetch the boat'); @@ -530,12 +534,12 @@ module('integration/unload - Unloading Records', function(hooks) { let boat = store.peekRecord('boat', '1'); let initialBoatInternalModel = boat._internalModel; let relationshipState = person.hasMany('boats').hasManyRelationship; - let knownPeople = store._internalModelsFor('person'); - let knownBoats = store._internalModelsFor('boat'); + let knownPeople = internalModelsFor(store, 'person'); + let knownBoats = internalModelsFor(store, 'boat'); // ensure we loaded the people and boats - assert.equal(knownPeople.models.length, 1, 'one person record is loaded'); - assert.equal(knownBoats.models.length, 1, 'one boat record is loaded'); + assert.equal(knownPeople.length, 1, 'one person record is loaded'); + assert.equal(knownBoats.length, 1, 'one boat record is loaded'); assert.equal(store.hasRecordForId('person', '1'), true); assert.equal(store.hasRecordForId('boat', '1'), true); @@ -554,8 +558,9 @@ module('integration/unload - Unloading Records', function(hooks) { }); // ensure that our new state is correct - assert.equal(knownPeople.models.length, 1, 'one person record is loaded'); - assert.equal(knownBoats.models.length, 0, 'no boat records are loaded'); + assert.equal(knownPeople.length, 1, 'one person record is loaded'); + assert.equal(knownBoats.length, 1, 'we still have a boat'); + assert.equal(knownBoats[0].isHiddenFromRecordArrays(), true, 'our boat is hidden'); assert.equal( relationshipState.canonicalMembers.size, 1, @@ -597,12 +602,12 @@ module('integration/unload - Unloading Records', function(hooks) { let boat = store.peekRecord('boat', '1'); let initialBoatInternalModel = boat._internalModel; let relationshipState = person.hasMany('boats').hasManyRelationship; - let knownPeople = store._internalModelsFor('person'); - let knownBoats = store._internalModelsFor('boat'); + let knownPeople = internalModelsFor(store, 'person'); + let knownBoats = internalModelsFor(store, 'boat'); // ensure we loaded the people and boats - assert.deepEqual(knownPeople.models.map(m => m.id), ['1'], 'one person record is loaded'); - assert.deepEqual(knownBoats.models.map(m => m.id), ['1'], 'one boat record is loaded'); + assert.deepEqual(knownPeople.map(m => m.id), ['1'], 'one person record is loaded'); + assert.deepEqual(knownBoats.map(m => m.id), ['1'], 'one boat record is loaded'); assert.equal(store.hasRecordForId('person', '1'), true); assert.equal(store.hasRecordForId('boat', '1'), true); @@ -627,9 +632,9 @@ module('integration/unload - Unloading Records', function(hooks) { run(() => boat.unloadRecord()); // ensure that our new state is correct - assert.deepEqual(knownPeople.models.map(m => m.id), ['1'], 'one person record is loaded'); - assert.deepEqual(knownBoats.models.map(m => m.id), ['1'], 'one boat record is known'); - assert.ok(knownBoats.models[0] === initialBoatInternalModel, 'We still have our boat'); + assert.deepEqual(knownPeople.map(m => m.id), ['1'], 'one person record is loaded'); + assert.deepEqual(knownBoats.map(m => m.id), ['1'], 'one boat record is known'); + assert.ok(knownBoats[0] === initialBoatInternalModel, 'We still have our boat'); assert.equal(initialBoatInternalModel.isEmpty(), true, 'Model is in the empty state'); assert.deepEqual( idsFromOrderedSet(relationshipState.canonicalMembers), @@ -752,11 +757,11 @@ module('integration/unload - Unloading Records', function(hooks) { }); assert.equal( - store._internalModelsFor('person').models.length, + internalModelsFor(store, 'person').length, 1, 'one person record is loaded' ); - assert.equal(store._internalModelsFor('boat').models.length, 2, 'two boat records are loaded'); + assert.equal(internalModelsFor(store, 'boat').length, 2, 'two boat records are loaded'); assert.equal(store.hasRecordForId('person', 1), true); assert.equal(store.hasRecordForId('boat', 1), true); assert.equal(store.hasRecordForId('boat', 2), true); @@ -795,8 +800,8 @@ module('integration/unload - Unloading Records', function(hooks) { store.peekRecord('boat', 2).unloadRecord(); }); - assert.equal(store._internalModelsFor('person').models.length, 0); - assert.equal(store._internalModelsFor('boat').models.length, 0); + assert.equal(internalModelsFor(store, 'person').length, 0); + assert.equal(internalModelsFor(store, 'boat').length, 0); assert.equal(checkOrphanCalls, 3, 'each internalModel checks for cleanup'); assert.equal(cleanupOrphanCalls, 3, 'each model data tries to cleanup'); diff --git a/tests/integration/relationships/belongs-to-test.js b/tests/integration/relationships/belongs-to-test.js index 77d8b5940d8..90202929e7f 100644 --- a/tests/integration/relationships/belongs-to-test.js +++ b/tests/integration/relationships/belongs-to-test.js @@ -1910,8 +1910,8 @@ test("belongsTo relationship with links doesn't trigger extra change notificatio test("belongsTo relationship doesn't trigger when model data doesn't support implicit relationship", function(assert) { class TestRecordData extends RecordData { - constructor(modelName, id, clientId, storeWrapper, store) { - super(modelName, id, clientId, storeWrapper, store); + constructor(identifier, storeWrapper) { + super(identifier, storeWrapper); delete this.__implicitRelationships; delete this.__relationships; } @@ -1928,7 +1928,7 @@ test("belongsTo relationship doesn't trigger when model data doesn't support imp destroy() { this.isDestroyed = true; - this.storeWrapper.disconnectRecord(this.modelName, this.id, this.clientId); + this.storeWrapper.disconnectRecord(this.modelName, this.id, this.lid); } get _implicitRelationships() { @@ -1947,11 +1947,13 @@ test("belongsTo relationship doesn't trigger when model data doesn't support imp }); const createRecordDataFor = env.store.createRecordDataFor; - env.store.createRecordDataFor = function(modelName, id, lid, storeWrapper) { - if (modelName === 'book1' || modelName === 'section') { - return new TestRecordData(modelName, id, lid, storeWrapper, this); + env.store.createRecordDataFor = function(type, id, lid, storeWrapper) { + if (type === 'book1' || type === 'section') { + let identifier = storeWrapper.store.identifierCache.getOrCreateRecordIdentifier({ type, id, lid }); + + return new TestRecordData(identifier, storeWrapper); } - return createRecordDataFor.call(this, modelName, id, lid, storeWrapper); + return createRecordDataFor.call(this, type, id, lid, storeWrapper); }; const data = { diff --git a/tests/integration/snapshot-test.js b/tests/integration/snapshot-test.js index 5513e86c5e8..c21f3c236db 100644 --- a/tests/integration/snapshot-test.js +++ b/tests/integration/snapshot-test.js @@ -9,6 +9,11 @@ const { Model, attr, hasMany, belongsTo, Snapshot } = DS; let env, Post, Comment; +function internalModelFor(store, type, id) { + let identifier = store.identifierCache.getOrCreateRecordIdentifier({ type, id }); + return store._imCache.ensureInstance(identifier); +} + module('integration/snapshot - Snapshot', { beforeEach() { Post = Model.extend({ @@ -122,7 +127,7 @@ test('snapshot.type loads the class lazily', function(assert) { }, }, }); - let postInternalModel = env.store._internalModelForId('post', 1); + let postInternalModel = internalModelFor(env.store, 'post', 1); let snapshot = postInternalModel.createSnapshot(); assert.equal(false, postClassLoaded, 'model class is not eagerly loaded'); @@ -165,7 +170,7 @@ test('snapshots for un-materialized internal-models generate attributes lazily', }) ); - let postInternalModel = env.store._internalModelForId('post', 1); + let postInternalModel = internalModelFor(env.store, 'post', 1); let snapshot = postInternalModel.createSnapshot(); let expected = { author: undefined, @@ -192,7 +197,7 @@ test('snapshots for materialized internal-models generate attributes greedily', }) ); - let postInternalModel = env.store._internalModelForId('post', 1); + let postInternalModel = internalModelFor(env.store, 'post', 1); let snapshot = postInternalModel.createSnapshot(); let expected = { author: undefined, diff --git a/tests/integration/store-test.js b/tests/integration/store-test.js index 9ef2e293ee6..4fe11ded0dd 100644 --- a/tests/integration/store-test.js +++ b/tests/integration/store-test.js @@ -922,7 +922,8 @@ test('Using store#fetch on an empty record calls find', function(assert) { }); }); - let car = store.recordForId('car', 20); + let identifier = store.identifierCache.getOrCreateRecordIdentifier({ type: 'car', id: '20' }); + let car = store._imCache.ensureInstance(identifier).getRecord(); assert.ok(car.get('isEmpty'), 'Car with id=20 should be empty'); return run(() => { diff --git a/tests/unit/model-test.js b/tests/unit/model-test.js index de1eeee299e..17595af92b2 100644 --- a/tests/unit/model-test.js +++ b/tests/unit/model-test.js @@ -344,7 +344,7 @@ module('unit/model - Model', function(hooks) { // test .get access of id assert.equal(person.get('id'), null, 'initial created model id should be null'); - store.setRecordId('odd-person', 'john', person._internalModel.clientId); + store.setRecordId('odd-person', 'john', person._internalModel.lid); oddId = person.get('idComputed'); assert.equal(oddId, 'john', 'computed get is correct'); diff --git a/tests/unit/model/lifecycle-callbacks-test.js b/tests/unit/model/lifecycle-callbacks-test.js index ac3aab1ada3..606b64879a9 100644 --- a/tests/unit/model/lifecycle-callbacks-test.js +++ b/tests/unit/model/lifecycle-callbacks-test.js @@ -53,7 +53,7 @@ test(`TEMPORARY: a record receives a didLoad callback once it materializes if it }); run(() => { - store._pushInternalModel({ id: 1, type: 'person' }); + store._pushResource({ id: 1, type: 'person' }); assert.equal(didLoadCalled, 0, 'didLoad was not called'); }); run(() => store.peekRecord('person', 1)); diff --git a/tests/unit/store/adapter-interop-test.js b/tests/unit/store/adapter-interop-test.js index 5e753025617..d3acbcd9e84 100644 --- a/tests/unit/store/adapter-interop-test.js +++ b/tests/unit/store/adapter-interop-test.js @@ -12,6 +12,11 @@ import DS from 'ember-data'; let TestAdapter, store; +function internalModelFor(store, type, id) { + let identifier = store.identifierCache.getOrCreateRecordIdentifier({ type, id }); + return store._imCache.ensureInstance(identifier); +} + module('unit/store/adapter-interop - DS.Store working with a DS.Adapter', { beforeEach() { TestAdapter = DS.Adapter.extend(); @@ -740,9 +745,9 @@ test('store._scheduleFetchMany should not resolve until all the records are reso store.createRecord('test'); let internalModels = [ - store._internalModelForId('test', 10), - store._internalModelForId('phone', 20), - store._internalModelForId('phone', 21), + internalModelFor(store, 'test', 10), + internalModelFor(store, 'phone', 20), + internalModelFor(store, 'phone', 21), ]; return run(() => { @@ -784,9 +789,9 @@ test('the store calls adapter.findMany according to groupings returned by adapte }); let internalModels = [ - store._internalModelForId('test', 10), - store._internalModelForId('test', 20), - store._internalModelForId('test', 21), + internalModelFor(store, 'test', 10), + internalModelFor(store, 'test', 20), + internalModelFor(store, 'test', 21), ]; return run(() => { @@ -1370,22 +1375,6 @@ test('store should reload all records in the background when `shouldBackgroundRe return done; }); -testInDebug('store should assert of the user tries to call store.filter', function(assert) { - assert.expect(1); - - const Person = DS.Model.extend({ - name: DS.attr('string'), - }); - - store = createStore({ - person: Person, - }); - - assert.expectAssertion(() => { - run(() => store.filter('person', {})); - }, /The filter API has been moved to a plugin/); -}); - testInDebug('Calling adapterFor with a model class should assert', function(assert) { const Person = DS.Model.extend({ name: DS.attr('string'), diff --git a/tests/unit/store/asserts-test.js b/tests/unit/store/asserts-test.js index b3032c36934..3686b77396f 100644 --- a/tests/unit/store/asserts-test.js +++ b/tests/unit/store/asserts-test.js @@ -22,7 +22,6 @@ module('unit/store/asserts - DS.Store methods produce useful assertion messages' 'findByIds', 'peekRecord', 'hasRecordForId', - 'recordForId', 'query', 'queryRecord', 'findAll', @@ -54,7 +53,6 @@ module('unit/store/asserts - DS.Store methods produce useful assertion messages' 'getReference', 'peekRecord', 'hasRecordForId', - 'recordForId', 'findMany', 'findHasMany', 'findBelongsTo', diff --git a/tests/unit/store/push-test.js b/tests/unit/store/push-test.js index e4596766d31..22e9a653195 100644 --- a/tests/unit/store/push-test.js +++ b/tests/unit/store/push-test.js @@ -745,7 +745,7 @@ testInDebug('calling push with hasMany relationship the value must be an array', }); testInDebug('calling push with missing or invalid `id` throws assertion error', function(assert) { - let invalidValues = [{}, { id: null }, { id: '' }]; + let invalidValues = [{ type: 'person' }, { type: 'person', id: null }, { type: 'person', id: '' }]; assert.expect(invalidValues.length); diff --git a/tests/unit/store/unload-test.js b/tests/unit/store/unload-test.js index 2ba613a8ece..e42fee2dca5 100644 --- a/tests/unit/store/unload-test.js +++ b/tests/unit/store/unload-test.js @@ -111,7 +111,7 @@ test('unload a record', function(assert) { test('unload followed by create of the same type + id', function(assert) { let record = store.createRecord('record', { id: 1 }); - assert.ok(store.recordForId('record', 1) === record, 'record should exactly equal'); + assert.ok(store.peekRecord('record', 1) === record, 'record should exactly equal'); return run(() => { record.unloadRecord();