diff --git a/ember-data-types/q/record-data-store-wrapper.ts b/ember-data-types/q/record-data-store-wrapper.ts index d7eb499b823..65389c3be73 100644 --- a/ember-data-types/q/record-data-store-wrapper.ts +++ b/ember-data-types/q/record-data-store-wrapper.ts @@ -11,16 +11,16 @@ import { SchemaDefinitionService } from './schema-definition-service'; */ /** - * RecordDataStoreWrapper provides encapsulated API access to the minimal - * subset of the Store's functionality that cache (RecordData) implementations + * CacheStoreWrapper provides encapsulated API access to the minimal + * subset of the Store's functionality that cache implementations * should interact with. It is provided to the Store's `createRecordDataFor` - * hook. + * and `createCache` hooks. * * Cache implementations should not need more than this API provides. * * This class cannot be directly instantiated. * - * @class RecordDataStoreWrapper + * @class CacheStoreWrapper * @public */ export interface LegacyRecordDataStoreWrapper { diff --git a/packages/-ember-data/README.md b/packages/-ember-data/README.md index 1cb610cf319..89f03c81d9f 100644 --- a/packages/-ember-data/README.md +++ b/packages/-ember-data/README.md @@ -41,7 +41,7 @@ not wish to use `ember-data`, remove `ember-data` from your project's `package.j EmberData is organized into primitives that compose together via public APIs. - [@ember-data/store](https://github.com/emberjs/data/tree/master/packages/store) is the core and handles coordination -- [@ember-data/json-api](https://github.com/emberjs/data/tree/master/packages/json-api) is a resource cache for JSON:API structured data. It integrates with the store via the hook `createRecordDataFor` +- [@ember-data/json-api](https://github.com/emberjs/data/tree/master/packages/json-api) provides a resource cache for JSON:API structured data. It integrates with the store via the hook `createCache` - [@ember-data/model](https://github.com/emberjs/data/tree/master/packages/model) is a presentation layer, it integrates with the store via the hooks `instantiateRecord` and `teardownRecord`. - [@ember-data/adapter](https://github.com/emberjs/data/tree/master/packages/adapter) provides various network API integrations for APIS built over specific REST or JSON:API conventions. - [@ember-data/serializer](https://github.com/emberjs/data/tree/master/packages/serializer) pairs with `@ember-data/adapter` to normalize and serialize data to and from an API format into the `JSON:API` format understood by `@ember-data/json-api`. diff --git a/packages/-ember-data/addon/index.js b/packages/-ember-data/addon/index.js index abd0831cdaa..0c49f0c67bd 100644 --- a/packages/-ember-data/addon/index.js +++ b/packages/-ember-data/addon/index.js @@ -72,7 +72,7 @@ not wish to use `ember-data`, remove `ember-data` from your project's `package.j *Ember*‍**Data** is organized into primitives that compose together via public APIs. - [@ember-data/store](/ember-data/release/modules/@ember-data%2Fstore) is the core and handles coordination -- [@ember-data/json-api](/ember-data/release/modules/@ember-data%2Frecord-data) is a resource cache for JSON:API structured data. It integrates with the store via the hook `createRecordDataFor` +- [@ember-data/json-api](/ember-data/release/modules/@ember-data%2Frecord-data) provides a resource cache for JSON:API structured data. It integrates with the store via the hook `createCache` - [@ember-data/model](/ember-data/release/modules/@ember-data%2Fmodel) is a presentation layer, it integrates with the store via the hooks `instantiateRecord` and `teardownRecord`. - [@ember-data/adapter](/ember-data/release/modules/@ember-data%2Fadapter) provides various network API integrations for APIS built over specific REST or JSON:API conventions. - [@ember-data/serializer](/ember-data/release/modules/@ember-data%2Fserializer) pairs with `@ember-data/adapter` to normalize and serialize data to and from an API format into the `JSON:API` format understood by `@ember-data/json-api`. diff --git a/packages/json-api/addon/-private/cache.ts b/packages/json-api/addon/-private/cache.ts index e64d1ba7323..5c90a049c71 100644 --- a/packages/json-api/addon/-private/cache.ts +++ b/packages/json-api/addon/-private/cache.ts @@ -36,7 +36,7 @@ const EMPTY_ITERATOR = { /** The default cache implementation used by ember-data. The cache is configurable and using a different implementation can be - achieved by implementing the store's createRecordDataFor hook. + achieved by implementing the store's createCache hook. @class RecordDataDefault @public @@ -78,23 +78,28 @@ export default class Cache implements CacheInterface { } /** - * Private method used when the store's `createRecordDataFor` hook is called - * to populate an entry for the identifier into the singleton. + * Private method used to populate an entry for the identifier + * into the singleton. * * @method createCache - * @private + * @internal * @param identifier */ - createCache(identifier: StableRecordIdentifier): void { + _createCache(identifier: StableRecordIdentifier): void { assert(`Expected no resource data to yet exist in the cache`, !this.__cache.has(identifier)); this.__cache.set(identifier, makeCache()); } - __peek(identifier: StableRecordIdentifier, allowDestroyed = false): CachedResource { + __safePeek(identifier: StableRecordIdentifier, allowDestroyed: boolean): CachedResource | undefined { let resource = this.__cache.get(identifier); if (!resource && allowDestroyed) { resource = this.__destroyedCache.get(identifier); } + return resource; + } + + __peek(identifier: StableRecordIdentifier, allowDestroyed: boolean): CachedResource { + let resource = this.__safePeek(identifier, allowDestroyed); assert( `Expected RecordData Cache to have a resource cache for the identifier ${String(identifier)} but none was found`, resource @@ -109,9 +114,9 @@ export default class Cache implements CacheInterface { ): void | string[] { let changedKeys: string[] | undefined; if (!this.__cache.has(identifier)) { - this.createCache(identifier); + this._createCache(identifier); } - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); if (LOG_OPERATIONS) { try { @@ -197,8 +202,8 @@ export default class Cache implements CacheInterface { console.log(`EmberData | Mutation - clientDidCreate ${identifier.lid}`, options); } } - this.createCache(identifier); - const cached = this.__peek(identifier); + this._createCache(identifier); + const cached = this.__peek(identifier, false); cached.isNew = true; let createOptions = {}; @@ -258,12 +263,12 @@ export default class Cache implements CacheInterface { return createOptions; } willCommit(identifier: StableRecordIdentifier): void { - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); cached.inflightAttrs = cached.localAttrs; cached.localAttrs = null; } didCommit(identifier: StableRecordIdentifier, data: JsonApiResource | null): void { - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); if (cached.isDeleted) { graphFor(this.__storeWrapper).push({ op: 'deleteRecord', @@ -305,7 +310,7 @@ export default class Cache implements CacheInterface { } commitWasRejected(identifier: StableRecordIdentifier, errors?: JsonApiValidationError[] | undefined): void { - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); if (cached.inflightAttrs) { let keys = Object.keys(cached.inflightAttrs); if (keys.length > 0) { @@ -325,7 +330,14 @@ export default class Cache implements CacheInterface { } unloadRecord(identifier: StableRecordIdentifier): void { - const cached = this.__peek(identifier); + // TODO this is necessary because + // we maintain memebership inside InstanceCache + // for peekAll, so even though we haven't created + // any data we think this exists. + if (!this.__cache.has(identifier)) { + return; + } + const cached = this.__peek(identifier, false); const storeWrapper = this.__storeWrapper; graphFor(storeWrapper).unload(identifier); @@ -382,7 +394,7 @@ export default class Cache implements CacheInterface { } } setAttr(identifier: StableRecordIdentifier, attr: string, value: unknown): void { - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); const existing = cached.inflightAttrs && attr in cached.inflightAttrs ? cached.inflightAttrs[attr] @@ -403,14 +415,14 @@ export default class Cache implements CacheInterface { } changedAttrs(identifier: StableRecordIdentifier): ChangedAttributesHash { // TODO freeze in dev - return (this.__peek(identifier).changes || Object.create(null)) as ChangedAttributesHash; + return (this.__peek(identifier, false).changes || Object.create(null)) as ChangedAttributesHash; } hasChangedAttrs(identifier: StableRecordIdentifier): boolean { const cached = this.__peek(identifier, true); return cached.localAttrs !== null && Object.keys(cached.localAttrs).length > 0; } rollbackAttrs(identifier: StableRecordIdentifier): string[] { - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); let dirtyKeys: string[] | undefined; cached.isDeleted = false; @@ -454,7 +466,7 @@ export default class Cache implements CacheInterface { } setIsDeleted(identifier: StableRecordIdentifier, isDeleted: boolean): void { - const cached = this.__peek(identifier); + const cached = this.__peek(identifier, false); cached.isDeleted = isDeleted; if (cached.isNew) { // TODO can we delete this since we will do this in unload? @@ -470,23 +482,17 @@ export default class Cache implements CacheInterface { return this.__peek(identifier, true).errors || []; } isEmpty(identifier: StableRecordIdentifier): boolean { - if (!this.__cache.has(identifier)) { - return true; - } - const cached = this.__peek(identifier, true); - return cached.remoteAttrs === null && cached.inflightAttrs === null && cached.localAttrs === null; + const cached = this.__safePeek(identifier, true); + return cached ? cached.remoteAttrs === null && cached.inflightAttrs === null && cached.localAttrs === null : true; } isNew(identifier: StableRecordIdentifier): boolean { - if (!this.__cache.has(identifier)) { - return false; - } - return this.__peek(identifier, true).isNew; + return this.__safePeek(identifier, true)?.isNew || false; } isDeleted(identifier: StableRecordIdentifier): boolean { - return this.__peek(identifier, true).isDeleted; + return this.__safePeek(identifier, true)?.isDeleted || false; } isDeletionCommitted(identifier: StableRecordIdentifier): boolean { - return this.__peek(identifier, true).isDeletionCommitted; + return this.__safePeek(identifier, true)?.isDeletionCommitted || false; } } diff --git a/packages/store/addon/-private/managers/record-data-store-wrapper.ts b/packages/store/addon/-private/managers/record-data-store-wrapper.ts index c0c44d71729..301d213caf5 100644 --- a/packages/store/addon/-private/managers/record-data-store-wrapper.ts +++ b/packages/store/addon/-private/managers/record-data-store-wrapper.ts @@ -246,9 +246,11 @@ class LegacyWrapper implements LegacyRecordDataStoreWrapper { ? this._store._instanceCache.getRecordData(identifier) : this._store.cache; - if (!id && !lid) { - cache.clientDidCreate(identifier); - this._store.recordArrayManager.identifierAdded(identifier); + if (DEPRECATE_V1CACHE_STORE_APIS) { + if (!id && !lid && typeof type === 'string') { + cache.clientDidCreate(identifier); + this._store.recordArrayManager.identifierAdded(identifier); + } } return cache; diff --git a/packages/store/addon/-private/store-service.ts b/packages/store/addon/-private/store-service.ts index bd95ce6c353..36a58feba31 100644 --- a/packages/store/addon/-private/store-service.ts +++ b/packages/store/addon/-private/store-service.ts @@ -168,6 +168,10 @@ export interface CreateRecordProperties { @extends Ember.Service */ +interface Store { + createRecordDataFor?(identifier: StableRecordIdentifier, wrapper: CacheStoreWrapper): Cache; +} + class Store extends Service { declare recordArrayManager: RecordArrayManager; @@ -2406,6 +2410,7 @@ class Store extends Service { * @method createCache (hook) * @public * @param storeWrapper + * @returns {Cache} */ createCache(storeWrapper: CacheStoreWrapper): Cache { if (HAS_JSON_API_PACKAGE) { @@ -2443,9 +2448,8 @@ class Store extends Service { * @public * @param identifier * @param storeWrapper - * @returns {RecordData} + * @returns {Cache} */ - declare createRecordDataFor: (identifier: StableRecordIdentifier, wrapper: CacheStoreWrapper) => Cache; /** `normalize` converts a json payload into the normalized form that diff --git a/tests/docs/fixtures/expected.js b/tests/docs/fixtures/expected.js index db1bfc0c80c..d127e30725d 100644 --- a/tests/docs/fixtures/expected.js +++ b/tests/docs/fixtures/expected.js @@ -401,6 +401,7 @@ module.exports = { '(public) @ember-data/store Store#unloadAll', '(public) @ember-data/store Store#unloadRecord', '(public) @ember-data/store Store#saveRecord', + '(public) @ember-data/store Store#createCache (hook)', '(public) @ember-data/store Store#createRecordDataFor (hook)', '(public) @ember-data/store Store#instantiateRecord (hook)', '(public) @ember-data/store Store#teardownRecord (hook)', diff --git a/tests/main/tests/integration/record-data/record-data-test.ts b/tests/main/tests/integration/record-data/record-data-test.ts index c330e6d8d43..471557b0979 100644 --- a/tests/main/tests/integration/record-data/record-data-test.ts +++ b/tests/main/tests/integration/record-data/record-data-test.ts @@ -567,7 +567,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( if (identifier.type === 'house') { return new RelationshipRecordData(storeWrapper, identifier); } else { - return this._super(identifier, storeWrapper); + return this.cache; } }, }); @@ -638,7 +638,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( if (identifier.type === 'house') { return new RelationshipRecordData(storeWrapper, identifier); } else { - return this._super(identifier, storeWrapper); + return this.cache; } }, }); @@ -732,7 +732,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( houseIdentifier = identifier; return new RelationshipRecordData(storeWrapper, identifier); } else { - return this._super(identifier, storeWrapper); + return this.cache; } }, }); @@ -874,7 +874,7 @@ module('integration/record-data - Custom RecordData Implementations', function ( if (identifier.type === 'house') { return new RelationshipRecordData(storeWrapper, identifier); } else { - return this._super(identifier, storeWrapper); + return this.cache; } }, }); diff --git a/tests/main/tests/integration/record-data/store-wrapper-test.ts b/tests/main/tests/integration/record-data/store-wrapper-test.ts index f60e5b3cf8d..53016d62020 100644 --- a/tests/main/tests/integration/record-data/store-wrapper-test.ts +++ b/tests/main/tests/integration/record-data/store-wrapper-test.ts @@ -261,7 +261,7 @@ module('integration/store-wrapper - RecordData StoreWrapper tests', function (ho if (identifier.type === 'house') { return new RecordDataForTest(identifier, wrapper); } else { - return this._super(identifier, wrapper); + return this.cache; } }, }); diff --git a/tests/main/tests/integration/record-data/unloading-record-data-test.js b/tests/main/tests/integration/record-data/unloading-record-data-test.js index 9fa679788aa..c4108f0de1a 100644 --- a/tests/main/tests/integration/record-data/unloading-record-data-test.js +++ b/tests/main/tests/integration/record-data/unloading-record-data-test.js @@ -173,7 +173,6 @@ module('RecordData Compatibility', function (hooks) { const CustomRecordData = DEPRECATE_V1_RECORD_DATA ? V1CustomRecordData : V2CustomRecordData; test(`store.unloadRecord on a record with default RecordData with relationship to a record with custom RecordData does not error`, async function (assert) { - const originalCreateRecordDataFor = store.createRecordDataFor; let customCalled = 0, customCalledFor = [], originalCalled = 0, @@ -186,7 +185,7 @@ module('RecordData Compatibility', function (hooks) { } else { originalCalled++; originalCalledFor.push(identifier); - return originalCreateRecordDataFor.call(this, identifier, storeWrapper); + return this.cache; } }; diff --git a/tests/main/tests/unit/custom-class-support/custom-class-model-test.ts b/tests/main/tests/unit/custom-class-support/custom-class-model-test.ts index 368f811299a..204c655c7ef 100644 --- a/tests/main/tests/unit/custom-class-support/custom-class-model-test.ts +++ b/tests/main/tests/unit/custom-class-support/custom-class-model-test.ts @@ -1,3 +1,5 @@ +import { TestContext } from '@ember/test-helpers'; + import { module, test } from 'qunit'; import RSVP from 'rsvp'; @@ -7,13 +9,13 @@ import JSONAPIAdapter from '@ember-data/adapter/json-api'; import JSONAPISerializer from '@ember-data/serializer/json-api'; import Store from '@ember-data/store'; import type { Snapshot } from '@ember-data/store/-private'; -import type NotificationManager from '@ember-data/store/-private/managers/record-notification-manager'; import type { RecordIdentifier, StableRecordIdentifier } from '@ember-data/types/q/identifier'; import { Cache } from '@ember-data/types/q/record-data'; import type { AttributesSchema, RelationshipsSchema } from '@ember-data/types/q/record-data-schemas'; import type { CacheStoreWrapper } from '@ember-data/types/q/record-data-store-wrapper'; import type { RecordInstance } from '@ember-data/types/q/record-instance'; import type { SchemaDefinitionService } from '@ember-data/types/q/schema-definition-service'; +import { deprecatedTest } from '@ember-data/unpublished-test-infra/test-support/deprecated-test'; module('unit/model - Custom Class Model', function (hooks) { let store: Store; @@ -77,19 +79,14 @@ module('unit/model - Custom Class Model', function (hooks) { let identifier; let storeWrapper; class CreationStore extends CustomStore { - createRecordDataFor(identifier: StableRecordIdentifier, sw: CacheStoreWrapper) { - let rd = super.createRecordDataFor(identifier, sw); + createCache(sw: CacheStoreWrapper) { + let rd = super.createCache(sw); storeWrapper = sw; return rd; } - instantiateRecord( - id: StableRecordIdentifier, - createRecordArgs, - recordDataFor, - notificationManager: NotificationManager - ): Object { + instantiateRecord(id: StableRecordIdentifier, createRecordArgs): Object { identifier = id; - notificationManager.subscribe(identifier, (passedId, key) => { + this.notifications.subscribe(identifier, (passedId, key) => { notificationCount++; assert.strictEqual(passedId, identifier, 'passed the identifier to the callback'); if (notificationCount === 1) { @@ -121,7 +118,7 @@ module('unit/model - Custom Class Model', function (hooks) { assert.expect(5); let returnValue; class CreationStore extends CustomStore { - instantiateRecord(identifier, createRecordArgs, recordDataFor, notificationManager) { + instantiateRecord(identifier, createRecordArgs) { assert.strictEqual(identifier.type, 'person', 'Identifier type passed in correctly'); assert.deepEqual(createRecordArgs, { otherProp: 'unk' }, 'createRecordArg passed in'); returnValue = {}; @@ -138,41 +135,46 @@ module('unit/model - Custom Class Model', function (hooks) { assert.deepEqual(returnValue, person, 'record instantiating does not modify the returned value'); }); - test('recordData lookup', function (assert) { - assert.expect(1); - let rd; - class CreationStore extends Store { - instantiateRecord(identifier, createRecordArgs, recordDataFor, notificationManager) { - rd = recordDataFor(identifier); - assert.strictEqual(rd.getAttr(identifier, 'name'), 'chris', 'Can look up record data from recordDataFor'); - return {}; + deprecatedTest( + 'recordData lookup', + { id: 'ember-data:deprecate-instantiate-record-args', count: 1, until: '5.0' }, + function (this: TestContext, assert: Assert) { + assert.expect(1); + let rd; + class CreationStore extends Store { + // @ts-expect-error + instantiateRecord(identifier: StableRecordIdentifier, createRecordArgs, recordDataFor): RecordInstance { + rd = recordDataFor(identifier); + assert.strictEqual(rd.getAttr(identifier, 'name'), 'chris', 'Can look up record data from recordDataFor'); + return {}; + } + teardownRecord(record) {} } - teardownRecord(record) {} - } - this.owner.register('service:store', CreationStore); - store = this.owner.lookup('service:store') as Store; - let schema: SchemaDefinitionService = { - attributesDefinitionFor({ type: string }): AttributesSchema { - return { - name: { - type: 'string', - options: {}, - name: 'name', - kind: 'attribute', - }, - }; - }, - relationshipsDefinitionFor({ type: string }): RelationshipsSchema { - return {}; - }, - doesTypeExist() { - return true; - }, - }; - store.registerSchemaDefinitionService(schema); + this.owner.register('service:store', CreationStore); + store = this.owner.lookup('service:store') as Store; + let schema: SchemaDefinitionService = { + attributesDefinitionFor({ type: string }): AttributesSchema { + return { + name: { + type: 'string', + options: {}, + name: 'name', + kind: 'attribute', + }, + }; + }, + relationshipsDefinitionFor({ type: string }): RelationshipsSchema { + return {}; + }, + doesTypeExist() { + return true; + }, + }; + store.registerSchemaDefinitionService(schema); - store.createRecord('person', { name: 'chris' }); - }); + store.createRecord('person', { name: 'chris' }); + } + ); test('attribute and relationship with custom schema definition', async function (assert) { assert.expect(18); @@ -232,7 +234,7 @@ module('unit/model - Custom Class Model', function (hooks) { }) ); class CustomStore extends Store { - instantiateRecord(identifier, createOptions, recordDataFor, notificationManager) { + instantiateRecord(identifier, createOptions) { return new Person(this); } teardownRecord(record) {} @@ -316,7 +318,6 @@ module('unit/model - Custom Class Model', function (hooks) { }); test('store.deleteRecord', async function (assert) { - let rd: Cache; let ident: StableRecordIdentifier; assert.expect(9); this.owner.register( @@ -330,13 +331,12 @@ module('unit/model - Custom Class Model', function (hooks) { }) ); let CreationStore = CustomStore.extend({ - instantiateRecord(identifier, createRecordArgs, recordDataFor, notificationManager) { + instantiateRecord(identifier, createRecordArgs) { ident = identifier; - rd = recordDataFor(identifier); - assert.false(rd.isDeleted(identifier), 'we are not deleted when we start'); - notificationManager.subscribe(identifier, (passedId, key) => { + assert.false(this.cache.isDeleted(identifier), 'we are not deleted when we start'); + this.notifications.subscribe(identifier, (passedId, key) => { assert.strictEqual(key, 'state', 'state change to deleted has been notified'); - assert.true(recordDataFor(identifier).isDeleted(identifier), 'we have been marked as deleted'); + assert.true(this.cache.isDeleted(identifier), 'we have been marked as deleted'); }); return {}; }, @@ -348,9 +348,9 @@ module('unit/model - Custom Class Model', function (hooks) { store = this.owner.lookup('service:store') as Store; let person = store.push({ data: { type: 'person', id: '1', attributes: { name: 'chris' } } }); store.deleteRecord(person); - assert.true(rd!.isDeleted(ident!), 'record has been marked as deleted'); + assert.true(store.cache.isDeleted(ident!), 'record has been marked as deleted'); await store.saveRecord(person); - assert.true(rd!.isDeletionCommitted(ident!), 'deletion has been commited'); + assert.true(store.cache.isDeletionCommitted(ident!), 'deletion has been commited'); }); test('record serialize', function (assert) { @@ -365,7 +365,7 @@ module('unit/model - Custom Class Model', function (hooks) { }) ); class CustomStore extends Store { - instantiateRecord(identifier, createOptions, recordDataFor, notificationManager) { + instantiateRecord(identifier, createOptions) { return new Person(this); } teardownRecord(record) {}