From 80417a4a8cd1fb9cc9d045b4e0d0dcbc0e8e8372 Mon Sep 17 00:00:00 2001 From: Igor Terzic Date: Tue, 12 May 2020 09:12:14 -0700 Subject: [PATCH] temp Co-authored-by: igorT Co-authored-by: pete-the-pete <> --- .../integration/identifiers/cache-test.ts | 24 +++++++++++ .../store/addon/-private/identifiers/cache.ts | 40 ++++++++++++++----- .../system/store/internal-model-factory.ts | 9 ++++- 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/packages/-ember-data/tests/integration/identifiers/cache-test.ts b/packages/-ember-data/tests/integration/identifiers/cache-test.ts index 7f62680b0bc..58a88988d3a 100644 --- a/packages/-ember-data/tests/integration/identifiers/cache-test.ts +++ b/packages/-ember-data/tests/integration/identifiers/cache-test.ts @@ -54,5 +54,29 @@ module('Integration | Identifiers - cache', function(hooks) { 'getOrCreateRecordIdentifier() return identifier' ); }); + + test('identifiers are cached by lid and can be looked up by lid igor', async function(assert) { + const houseHash = { + type: 'house', + id: '1', + attributes: { + name: 'Moomin', + }, + }; + const cache = identifierCacheFor(store); + const identifier = cache.getOrCreateRecordIdentifier(houseHash); + + assert.strictEqual( + identifier, + cache.getOrCreateRecordIdentifier({lid: identifier.lid}), + 'getOrCreateRecordIdentifier() return identifier' + ); + + assert.strictEqual( + identifier, + cache.getOrCreateRecordIdentifier({ type: 'not house', lid: identifier.lid}), + 'getOrCreateRecordIdentifier() return identifier' + ); + }); }); }); diff --git a/packages/store/addon/-private/identifiers/cache.ts b/packages/store/addon/-private/identifiers/cache.ts index e31cdbbc2d6..bf6c6c9721c 100644 --- a/packages/store/addon/-private/identifiers/cache.ts +++ b/packages/store/addon/-private/identifiers/cache.ts @@ -3,7 +3,7 @@ import { DEBUG } from '@glimmer/env'; import coerceId from '../system/coerce-id'; import normalizeModelName from '../system/normalize-model-name'; -import { DEBUG_CLIENT_ORIGINATED, DEBUG_IDENTIFIER_BUCKET } from '../ts-interfaces/identifier'; +import { DEBUG_CLIENT_ORIGINATED, DEBUG_IDENTIFIER_BUCKET, Identifier } from '../ts-interfaces/identifier'; import { addSymbol } from '../ts-interfaces/utils/symbol'; import isNonEmptyString from '../utils/is-non-empty-string'; import isStableIdentifier, { markStableIdentifier, unmarkStableIdentifier } from './is-stable-identifier'; @@ -137,7 +137,7 @@ export class IdentifierCache { /** * @internal */ - private _getRecordIdentifier(resource: ResourceIdentifierObject, shouldGenerate: true): StableRecordIdentifier; + private _getRecordIdentifier(resource: ResourceIdentifierObject | Identifier, shouldGenerate: true): StableRecordIdentifier; private _getRecordIdentifier( resource: ResourceIdentifierObject, shouldGenerate: false @@ -157,18 +157,31 @@ export class IdentifierCache { return resource; } + let lid = coerceId(resource.lid); + let identifier: StableRecordIdentifier | undefined = lid !== null ? this._cache.lids[lid] : undefined; + + if (identifier !== undefined) { + return identifier; + } + + const _resource = resource as ResourceIdentifierObject; + let type = normalizeModelName(_resource.type); + let id = coerceId(_resource.id); + + if (shouldGenerate === false) { + if (!type || !id) { + return; + } + } + // `type` must always be present if (DEBUG) { - if (!isNonEmptyString(resource.type)) { + 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 | undefined; - let lid = coerceId(resource.lid); - let id = coerceId(resource.id); // go straight for the stable RecordIdentifier key'd to `lid` if (lid !== null) { @@ -264,7 +277,7 @@ export class IdentifierCache { `id` + `type` or `lid` will return the same `lid` value) - this referential stability of the object itself is guaranteed */ - getOrCreateRecordIdentifier(resource: ResourceIdentifierObject | ExistingResourceObject): StableRecordIdentifier { + getOrCreateRecordIdentifier(resource: ResourceIdentifierObject | ExistingResourceObject | Identifier): StableRecordIdentifier { return this._getRecordIdentifier(resource, true); } @@ -325,7 +338,7 @@ export class IdentifierCache { let newId = coerceId(data.id); const keyOptions = getTypeIndex(this._cache.types, identifier.type); - let existingIdentifier = detectMerge(keyOptions, identifier, data, newId, this._cache.lids); + let existingIdentifier = detectMerge(this._cache.types, identifier, data, newId, this._cache.lids); if (existingIdentifier) { identifier = this._mergeRecordIdentifiers(keyOptions, identifier, existingIdentifier, data, newId as string); @@ -519,7 +532,7 @@ function performRecordIdentifierUpdate( } function detectMerge( - keyOptions: KeyOptions, + typesCache: ConfidentDict, identifier: StableRecordIdentifier, data: ResourceIdentifierObject | ExistingResourceObject, newId: string | null, @@ -527,7 +540,8 @@ function detectMerge( ): StableRecordIdentifier | false { const { id, type, lid } = identifier; if (id !== null && id !== newId && newId !== null) { - const existingIdentifier = keyOptions.id[newId]; + let keyOptions = getTypeIndex(typesCache, identifier.type); + let existingIdentifier = keyOptions.id[newId]; return existingIdentifier !== undefined ? existingIdentifier : false; } else { @@ -536,6 +550,10 @@ function detectMerge( if (id !== null && id === newId && newType === type && data.lid && data.lid !== lid) { const existingIdentifier = lids[data.lid]; + return existingIdentifier !== undefined ? existingIdentifier : false; + } else if (id !== null && id === newId && newType !== type && data.lid && data.lid === lid) { + let keyOptions = getTypeIndex(typesCache, newType); + let existingIdentifier = keyOptions.id[id]; return existingIdentifier !== undefined ? existingIdentifier : false; } } diff --git a/packages/store/addon/-private/system/store/internal-model-factory.ts b/packages/store/addon/-private/system/store/internal-model-factory.ts index c36399ad4c4..e79d69cc648 100644 --- a/packages/store/addon/-private/system/store/internal-model-factory.ts +++ b/packages/store/addon/-private/system/store/internal-model-factory.ts @@ -81,8 +81,13 @@ export default class InternalModelFactory { constructor(public store: CoreStore) { this.identifierCache = identifierCacheFor(store); this.identifierCache.__configureMerge((identifier, matchedIdentifier, resourceData) => { - const intendedIdentifier = identifier.id === resourceData.id ? identifier : matchedIdentifier; - const altIdentifier = identifier.id === resourceData.id ? matchedIdentifier : identifier; + let intendedIdentifier = identifier; + if (identifier.id !== matchedIdentifier.id) { + intendedIdentifier = identifier.id === resourceData.id ? identifier : matchedIdentifier; + } else if (identifier.type !== matchedIdentifier.type) { + intendedIdentifier = identifier.type === resourceData.type ? identifier : matchedIdentifier; + } + let altIdentifier = identifier === intendedIdentifier ? matchedIdentifier : identifier; // check for duplicate InternalModel's const map = this.modelMapFor(identifier.type);