diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000000..182af92f740 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,43 @@ +{ + "eslint.format.enable": true, + "eslint.workingDirectories": [{ "mode": "auto" }, "packages/*", "tests/*"], + "eslint.options": { "reportUnusedDisableDirectives": "error" }, + "editor.codeActionsOnSave": { + "source.fixAll.eslint": "explicit" + }, + "files.associations": { + "turbo.json": "jsonc" + }, + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true, + "[javascript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true + }, + "[typescript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true + }, + "[handlebars]": { + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true + }, + "[json]": { + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true + }, + "[glimmer-js]": { + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true + }, + "[glimmer-ts]": { + "editor.defaultFormatter": "esbenp.prettier-vscode", + "editor.formatOnSave": true + }, + "[markdown]": { + "editor.detectIndentation": false, + "editor.insertSpaces": false, + "editor.tabSize": 4 + }, + "typescript.preferences.importModuleSpecifier": "project-relative" +} diff --git a/packages/store/src/-private/caches/identifier-cache.ts b/packages/store/src/-private/caches/identifier-cache.ts index ff579873b7d..239801dd635 100644 --- a/packages/store/src/-private/caches/identifier-cache.ts +++ b/packages/store/src/-private/caches/identifier-cache.ts @@ -84,6 +84,7 @@ type StableCache = { resources: IdentifierMap; documents: Map; resourcesByType: TypeMap; + polymorphicLidBackMap: Map; }; export type KeyInfoMethod = (resource: unknown, known: StableRecordIdentifier | null) => KeyInfo; @@ -251,6 +252,7 @@ export class IdentifierCache { resources: new Map(), resourcesByType: Object.create(null) as TypeMap, documents: new Map(), + polymorphicLidBackMap: new Map(), }; } @@ -560,16 +562,33 @@ export class IdentifierCache { const kept = this._merge(identifier, existingIdentifier, data); const abandoned = kept === identifier ? existingIdentifier : identifier; + // get any backreferences before forgetting this identifier, as it will be removed from the cache + // and we will no longer be able to find them + const abandonedBackReferences = this._cache.polymorphicLidBackMap.get(abandoned.lid); + // delete the backreferences for the abandoned identifier so that forgetRecordIdentifier + // does not try to remove them. + if (abandonedBackReferences) this._cache.polymorphicLidBackMap.delete(abandoned.lid); + // cleanup the identifier we no longer need this.forgetRecordIdentifier(abandoned); - // ensure a secondary cache entry for this id for the identifier we do keep - // keyOptions.id.set(newId, kept); + // ensure a secondary cache entry for the original lid for the abandoned identifier + this._cache.resources.set(abandoned.lid, kept); + + // backReferences let us know which other identifiers are pointing at this identifier + // so we can delete them later if we forget this identifier + const keptBackReferences = this._cache.polymorphicLidBackMap.get(kept.lid) ?? []; + keptBackReferences.push(abandoned.lid); - // ensure a secondary cache entry for this id for the abandoned identifier's type we do keep - // let baseKeyOptions = getTypeIndex(this._cache.resourcesByType, existingIdentifier.type); - // baseKeyOptions.id.set(newId, kept); + // update the backreferences from the abandoned identifier to be for the kept identifier + if (abandonedBackReferences) { + abandonedBackReferences.forEach((lid) => { + keptBackReferences.push(lid); + this._cache.resources.set(lid, kept); + }); + } + this._cache.polymorphicLidBackMap.set(kept.lid, keptBackReferences); return kept; } @@ -596,6 +615,14 @@ export class IdentifierCache { this._cache.resources.delete(identifier.lid); typeSet.lid.delete(identifier.lid); + const backReferences = this._cache.polymorphicLidBackMap.get(identifier.lid); + if (backReferences) { + backReferences.forEach((lid) => { + this._cache.resources.delete(lid); + }); + this._cache.polymorphicLidBackMap.delete(identifier.lid); + } + if (DEBUG) { identifier[DEBUG_STALE_CACHE_OWNER] = identifier[CACHE_OWNER]; } diff --git a/tests/main/tests/integration/identifiers/configuration-test.ts b/tests/main/tests/integration/identifiers/configuration-test.ts index ff675b8a8d8..efd9401abb6 100644 --- a/tests/main/tests/integration/identifiers/configuration-test.ts +++ b/tests/main/tests/integration/identifiers/configuration-test.ts @@ -383,6 +383,10 @@ module('Integration | Identifiers - configuration', function (hooks) { if (bucket !== 'record') { throw new Error('Test cannot generate an lid for a non-record'); } + if (typeof resource === 'object' && resource !== null && 'lid' in resource && typeof resource.lid === 'string') { + generateLidCalls++; + return resource.lid; + } if (typeof resource !== 'object' || resource === null || !('type' in resource)) { throw new Error('Test cannot generate an lid for a non-object'); } @@ -397,7 +401,7 @@ module('Integration | Identifiers - configuration', function (hooks) { let testMethod = (identifier) => { forgetMethodCalls++; - assert.strictEqual(expectedIdentifier, identifier, `We forgot the expected identifier ${expectedIdentifier}`); + assert.strictEqual(identifier, expectedIdentifier, `We forgot the expected identifier ${expectedIdentifier}`); }; setIdentifierForgetMethod((identifier) => { @@ -433,7 +437,11 @@ module('Integration | Identifiers - configuration', function (hooks) { const finalUserByIdIdentifier = recordIdentifierFor(userById); assert.strictEqual(generateLidCalls, 2, 'We generated no new lids when we looked up the originals'); - assert.strictEqual(store.identifierCache._cache.resources.size, 1, 'We now have only 1 identifier in the cache'); + assert.strictEqual( + store.identifierCache._cache.resources.size, + 2, + 'We keep a back reference identifier in the cache' + ); assert.strictEqual(forgetMethodCalls, 1, 'We abandoned an identifier'); assert.notStrictEqual( @@ -452,6 +460,36 @@ module('Integration | Identifiers - configuration', function (hooks) { 'We are using the identifier by id for the result of findRecord with username' ); + const recordA = store.peekRecord({ lid: finalUserByUsernameIdentifier.lid }); + const recordB = store.peekRecord({ lid: finalUserByIdIdentifier.lid }); + const recordC = store.peekRecord({ lid: originalUserByUsernameIdentifier.lid }); + const recordD = store.peekRecord('user', '@runspired'); + const recordE = store.peekRecord('user', '1'); + + assert.strictEqual(recordA, recordB, 'We have a single record for both identifiers'); + assert.strictEqual(recordA, recordC, 'We have a single record for both identifiers'); + assert.strictEqual(recordA, recordD, 'We have a single record for both identifiers'); + assert.strictEqual(recordA, recordE, 'We have a single record for both identifiers'); + + const regeneratedIdentifier = store.identifierCache.getOrCreateRecordIdentifier({ + lid: finalUserByUsernameIdentifier.lid, + }); + const regeneratedIdentifier2 = store.identifierCache.getOrCreateRecordIdentifier({ + id: '@runspired', + type: 'user', + }); + assert.strictEqual(regeneratedIdentifier, finalUserByUsernameIdentifier, 'We regenerate the same identifier'); + assert.strictEqual(regeneratedIdentifier2, finalUserByUsernameIdentifier, 'We regenerate the same identifier'); + + expectedIdentifier = finalUserByIdIdentifier; + store.unloadRecord(recordA); + await settled(); + assert.strictEqual( + store.identifierCache._cache.resources.size, + 0, + 'We have no identifiers or backreferences in the cache' + ); + // end test before store teardown testMethod = () => {}; }); diff --git a/tests/main/tests/integration/identifiers/scenarios-test.ts b/tests/main/tests/integration/identifiers/scenarios-test.ts index 98bae8dcfbd..7d892e28a2c 100644 --- a/tests/main/tests/integration/identifiers/scenarios-test.ts +++ b/tests/main/tests/integration/identifiers/scenarios-test.ts @@ -472,12 +472,14 @@ module('Integration | Identifiers - scenarios', function (hooks) { // ensure we truly are in a good state internally const lidCache = store.identifierCache._cache.resources; - const lids = [...lidCache.values()]; - assert.strictEqual( - lidCache.size, - 1, - `We only have the lid '${identifierByUsername.lid}' in ['${lids.join("', '")}']` + assert.strictEqual(lidCache.size, 2, `We should have both lids in the cache still since one is a backreference`); + assert.deepEqual( + [...lidCache.keys()], + ['remote:user:1:9001', 'remote:user:@runspired:9000'], + 'We have the expected keys' ); + const lids = [...lidCache.values()]; + assert.arrayStrictEquals(lids, [identifierById, identifierById], 'We have the expected values'); // ensure we still use secondary caching for @runspired post-merging of the identifiers const recordByUsernameAgain = (await store.findRecord('user', '@runspired')) as User; diff --git a/tests/main/tests/integration/records/polymorphic-find-record-test.ts b/tests/main/tests/integration/records/polymorphic-find-record-test.ts new file mode 100644 index 00000000000..1c52de5e36a --- /dev/null +++ b/tests/main/tests/integration/records/polymorphic-find-record-test.ts @@ -0,0 +1,53 @@ +import { settled } from '@ember/test-helpers'; + +import { module, test } from 'qunit'; + +import type Store from 'ember-data/store'; +import { setupTest } from 'ember-qunit'; + +import Model, { attr } from '@ember-data/model'; +import { recordIdentifierFor } from '@ember-data/store'; + +class Person extends Model { + @attr declare name: string; +} +class Employee extends Person {} + +module('integration/records/polymorphic-find-record - Polymorphic findRecord', function (hooks) { + setupTest(hooks); + + test('when findRecord with abstract type returns concrete type', async function (assert) { + this.owner.register('model:person', Person); + this.owner.register('model:employee', Employee); + + const store = this.owner.lookup('service:store') as Store; + const adapter = store.adapterFor('application'); + + adapter.findRecord = () => { + return Promise.resolve({ + data: { + id: '1', + type: 'employee', + attributes: { + name: 'Rey Skybarker', + }, + }, + }); + }; + + const person = (await store.findRecord('person', '1')) as Employee; + assert.ok(person instanceof Employee, 'record is an instance of Employee'); + assert.strictEqual(person.name, 'Rey Skybarker', 'name is correct'); + assert.strictEqual(recordIdentifierFor(person).type, 'employee', 'identifier has the concrete type'); + + const employee = store.peekRecord('employee', '1'); + const person2 = store.peekRecord('person', '1'); + assert.strictEqual(employee, person, 'peekRecord returns the same instance for concrete type'); + assert.strictEqual(person2, person, 'peekRecord returns the same instance for abstract type'); + assert.strictEqual(store.identifierCache._cache.resources.size, 2, 'identifier cache contains backreferences'); + + person.unloadRecord(); + await settled(); + assert.strictEqual(store.identifierCache._cache.resources.size, 0, 'identifier cache is empty'); + }); +});