From 4a4c31006b3b220cabf2ca1089dfc55fb7c7eafc Mon Sep 17 00:00:00 2001 From: Igor Terzic Date: Mon, 12 Aug 2019 11:14:05 -0700 Subject: [PATCH] id setting fixes serialize test more testing deleteRecord unloadRecord cleanup attr notifying ff manyArray change schema tests --- .../custom-class-model-test.ts | 160 ++++++++++++++++-- packages/serializer/addon/json-api.js | 3 +- .../store/addon/-private/system/core-store.ts | 20 ++- .../addon/-private/system/ds-model-store.ts | 14 +- .../store/addon/-private/system/many-array.js | 10 +- .../-private/system/model/internal-model.ts | 6 +- .../system/record-notification-manager.ts | 4 +- .../store/addon/-private/system/snapshot.js | 16 +- 8 files changed, 201 insertions(+), 32 deletions(-) diff --git a/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts b/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts index 189c9ffb866..761e268ba82 100644 --- a/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts +++ b/packages/-ember-data/tests/unit/custom-class-support/custom-class-model-test.ts @@ -10,9 +10,8 @@ import { InvalidError } from 'ember-data/adapters/errors'; import JSONAPIAdapter from 'ember-data/adapters/json-api'; import JSONAPISerializer from 'ember-data/serializers/json-api'; import JSONSerializer from 'ember-data/serializers/json'; -import { attr, hasMany, belongsTo } from '@ember-decorators/data'; import DSattr from 'ember-data/attr'; -import { recordDataFor } from 'ember-data/-private'; +import { recordDataFor, Snapshot } from 'ember-data/-private'; import Store from 'ember-data/store'; import { CUSTOM_MODEL_CLASS } from '@ember-data/canary-features'; import CoreStore from '@ember-data/store/-private/system/core-store'; @@ -29,8 +28,7 @@ if (CUSTOM_MODEL_CLASS) { let { owner } = this; class Person { - store: CoreStore; - constructor(store) { + constructor(public store: CoreStore) { this.store = store; } save() { @@ -120,12 +118,14 @@ if (CUSTOM_MODEL_CLASS) { }); test('record creation and teardown', function(assert) { - assert.expect(3); + assert.expect(5); + let returnValue; let CreationStore = CustomStore.extend({ instantiateRecord(identifier, createRecordArgs, recordDataFor, notificationManager) { assert.equal(identifier.type, 'person', 'Identifier type passed in correctly'); assert.deepEqual(createRecordArgs, { name: 'chris' }, 'createRecordArg passed in'); - return {}; + returnValue = {}; + return returnValue; }, teardownRecord(record) { assert.equal(record, person, 'Passed in person to teardown'); @@ -134,6 +134,8 @@ if (CUSTOM_MODEL_CLASS) { this.owner.register('service:store', CreationStore); store = this.owner.lookup('service:store'); let person = store.createRecord('person', { name: 'chris' }); + assert.equal(returnValue, person, 'createRecord returns the instantiated record'); + assert.deepEqual(returnValue, person, 'record instantiating does not modify the returned value'); }); test('recordData lookup igor5', function(assert) { @@ -154,26 +156,129 @@ if (CUSTOM_MODEL_CLASS) { let person = store.createRecord('person', { name: 'chris' }); }); - test('schema definition', function(assert) { + test('attribute and relationship with custom schema definition', async function(assert) { + assert.expect(10); this.owner.register( 'adapter:application', JSONAPIAdapter.extend({ shouldBackgroundReloadRecord: () => false, - createRecord: (store, type, snapshot) => { - return RSVP.reject(); + createRecord: (store, type, snapshot: Snapshot) => { + let count = 0; + snapshot.eachAttribute((attr, attrDef) => { + if (count === 0) { + assert.equal(attr, 'name', 'attribute key is correct'); + assert.deepEqual(attrDef, { type: 'string', key: 'name', name: 'name' }, 'attribute def matches schem'); + } else if (count === 1) { + assert.equal(attr, 'age', 'attribute key is correct'); + assert.deepEqual(attrDef, { type: 'number', key: 'age', name: 'age' }, 'attribute def matches schem'); + } + count++; + }); + count = 0; + snapshot.eachRelationship((rel, relDef) => { + if (count === 0) { + assert.equal(rel, 'boats', 'relationship key is correct'); + assert.deepEqual( + relDef, + { + type: 'ship', + kind: 'hasMany', + inverse: null, + options: {}, + key: 'boats', + }, + 'relationships def matches schem' + ); + } else if (count === 1) { + assert.equal(rel, 'house', 'relationship key is correct'); + assert.deepEqual( + relDef, + { type: 'house', kind: 'belongsTo', inverse: null, options: {}, key: 'house', name: 'house' }, + 'relationship def matches schem' + ); + } + count++; + }); + return RSVP.resolve({ data: { type: 'person', id: '1' } }); }, }) ); this.owner.register('service:store', CustomStore); store = this.owner.lookup('service:store'); - store.registerSchemaDefinitionService(schemaDefinition); - assert.expect(1); + let schema = { + attributesDefinitonFor(modelName: string) { + assert.equal(modelName, 'person', 'type passed in to the schema hooks'); + return { + name: { + type: 'string', + key: 'name', + name: 'name', + }, + age: { + type: 'number', + key: 'age', + name: 'age', + }, + }; + }, + relationshipsDefinitionFor(modelName: string) { + assert.equal(modelName, 'person', 'type passed in to the schema hooks'); + return { + boats: { + type: 'ship', + kind: 'hasMany', + inverse: null, + options: {}, + key: 'boats', + }, + house: { + type: 'house', + kind: 'belongsTo', + inverse: null, + options: {}, + key: 'house', + name: 'house', + }, + }; + }, + doesTypeExist() { + return true; + }, + }; + store.registerSchemaDefinitionService(schema); let person = store.createRecord('person', { name: 'chris' }); - person.save(); - assert.ok(true); + await person.save(); + }); + + test('hasModelFor with custom schema definition igor9', async function(assert) { + assert.expect(4); + this.owner.register('service:store', CustomStore); + store = this.owner.lookup('service:store'); + let count = 0; + let schema = { + attributesDefinitonFor() { + return {}; + }, + relationshipsDefinitionFor() { + return {}; + }, + doesTypeExist(modelName: string) { + if (count === 0) { + assert.equal(modelName, 'person', 'type passed in to the schema hooks'); + } else if (count === 1) { + assert.equal(modelName, 'boat', 'type passed in to the schema hooks'); + } + count++; + return modelName === 'person'; + }, + }; + store.registerSchemaDefinitionService(schema); + assert.equal(store._hasModelFor('person'), true, 'hasModelFor matches schema hook when true'); + assert.equal(store._hasModelFor('boat'), false, 'hasModelFor matches schema hook when false'); }); test('record saving', async function(assert) { + assert.expect(1); this.owner.register( 'adapter:application', JSONAPIAdapter.extend({ @@ -185,13 +290,12 @@ if (CUSTOM_MODEL_CLASS) { ); this.owner.register('service:store', CustomStore); store = this.owner.lookup('service:store'); - assert.expect(1); let person = store.createRecord('person', { name: 'chris' }); let promisePerson = await store.saveRecord(person); assert.equal(person, promisePerson, 'save promise resolves with the same record'); }); - test('record serialize igor8', function(assert) { + test('record serialize', function(assert) { assert.expect(1); this.owner.register( 'adapter:application', @@ -210,6 +314,8 @@ if (CUSTOM_MODEL_CLASS) { return { name: { type: 'string', + key: 'name', + name: 'name', }, }; } else if (modelName === 'house') { @@ -249,8 +355,28 @@ if (CUSTOM_MODEL_CLASS) { relationships: { house: { data: { type: 'house', id: '1' } } }, }, }); - let result = store.serializeRecord(person); - debugger; + let serialized = store.serializeRecord(person, { includeId: true }); + assert.deepEqual( + { + data: { + id: '7', + type: 'people', + attributes: { + name: 'chris', + }, + relationships: { + house: { + data: { + type: 'houses', + id: '1', + }, + }, + }, + }, + }, + serialized, + 'serializes record correctly' + ); }); }); } diff --git a/packages/serializer/addon/json-api.js b/packages/serializer/addon/json-api.js index ab663c1d4cc..99f6374255f 100644 --- a/packages/serializer/addon/json-api.js +++ b/packages/serializer/addon/json-api.js @@ -467,6 +467,7 @@ const JSONAPISerializer = JSONSerializer.extend({ }, serializeAttribute(snapshot, json, key, attribute) { + debugger; let type = attribute.type; if (this._canSerialize(key)) { @@ -493,7 +494,7 @@ const JSONAPISerializer = JSONSerializer.extend({ if (this._canSerialize(key)) { let belongsTo = snapshot.belongsTo(key); - let belongsToIsNotNew = belongsTo && belongsTo.record && !belongsTo.record.get('isNew'); + let belongsToIsNotNew = belongsTo && belongsTo.record && !belongsTo.isNew; if (belongsTo === null || belongsToIsNotNew) { json.relationships = json.relationships || {}; diff --git a/packages/store/addon/-private/system/core-store.ts b/packages/store/addon/-private/system/core-store.ts index 4ef49a4a540..320d94e5930 100644 --- a/packages/store/addon/-private/system/core-store.ts +++ b/packages/store/addon/-private/system/core-store.ts @@ -616,9 +616,13 @@ abstract class CoreStore extends Service { assertDestroyingStore(this, 'deleteRecord'); } if (CUSTOM_MODEL_CLASS) { - let identifier = recordIdentifierFor(record); - let internalModel = internalModelFactoryFor(this).peek(identifier.type, identifier.id, identifier.lid); - internalModel!.deleteRecord(); + if (record instanceof Model) { + return record.deleteRecord(); + } else { + let identifier = recordIdentifierFor(record); + let internalModel = internalModelFactoryFor(this).peek(identifier.type, identifier.id, identifier.lid); + internalModel!.deleteRecord(); + } } else { record.deleteRecord(); } @@ -644,9 +648,13 @@ abstract class CoreStore extends Service { assertDestroyingStore(this, 'unloadRecord'); } if (CUSTOM_MODEL_CLASS) { - let identifier = recordIdentifierFor(record); - let internalModel = internalModelFactoryFor(this).peek(identifier.type, identifier.id, identifier.lid); - internalModel!.unloadRecord(); + if (record instanceof Model) { + return record.unloadRecord(); + } else { + let identifier = recordIdentifierFor(record); + let internalModel = internalModelFactoryFor(this).peek(identifier.type, identifier.id, identifier.lid); + internalModel!.unloadRecord(); + } } else { record.unloadRecord(); } diff --git a/packages/store/addon/-private/system/ds-model-store.ts b/packages/store/addon/-private/system/ds-model-store.ts index 7f7ed2a6326..2784fcabe45 100644 --- a/packages/store/addon/-private/system/ds-model-store.ts +++ b/packages/store/addon/-private/system/ds-model-store.ts @@ -22,6 +22,7 @@ import { Identifier } from 'estree'; import { RecordData } from '..'; import RecordDataRecordWrapper from '../ts-interfaces/record-data-record-wrapper'; import { relationshipFromMeta } from './relationship-meta'; +import { cacheFor } from '@ember/object/internals'; // Implementors Note: // @@ -113,12 +114,19 @@ import { relationshipFromMeta } from './relationship-meta'; */ function notifyChanges( identifier: RecordIdentifier, - value: 'attributes' | 'relationships' | 'errors' | 'meta' | 'unload' | 'property' | 'state', + value: 'attributes' | 'relationships' | 'errors' | 'meta' | 'unload' | 'identity' | 'property' | 'state', record: DSModel, store: Store ) { if (value === 'attributes') { - record.eachAttribute(key => record.notifyPropertyChange(key)); + record.eachAttribute(key => { + //TODO don't always get if we haven't gotten before + let currentValue = cacheFor(record, key); + let internalModel = store._internalModelForResource(identifier); + if (currentValue !== internalModel._recordData.getAttr(key)) { + record.notifyPropertyChange(key); + } + }); } else if (value === 'relationships') { record.eachRelationship((key, meta) => { let internalModel = store._internalModelForResource(identifier); @@ -142,6 +150,8 @@ function notifyChanges( } else if (value === 'state') { record.notifyPropertyChange('isNew'); record.notifyPropertyChange('isDeleted'); + } else if (value === 'identity') { + record.notifyPropertyChange('id'); } } diff --git a/packages/store/addon/-private/system/many-array.js b/packages/store/addon/-private/system/many-array.js index 15496e0159a..c260c207607 100644 --- a/packages/store/addon/-private/system/many-array.js +++ b/packages/store/addon/-private/system/many-array.js @@ -13,7 +13,7 @@ import { PromiseArray } from './promise-proxies'; import { _objectIsAlive } from './store/common'; import diffArray from './diff-array'; import recordDataFor from './record-data-for'; - +import { CUSTOM_MODEL_CLASS } from '@ember-data/canary-features'; /** A `ManyArray` is a `MutableArray` that represents the contents of a has-many relationship. @@ -150,7 +150,13 @@ export default EmberObject.extend(MutableArray, DeprecatedEvent, { removeUnloadedInternalModel() { for (let i = 0; i < this.currentState.length; ++i) { let internalModel = this.currentState[i]; - if (internalModel._isDematerializing) { + let shouldRemove; + if (CUSTOM_MODEL_CLASS) { + shouldRemove = internalModel._isDematerializing; + } else { + shouldRemove = internalModel._isDematerializing || internalModel.isLoaded(); + } + if (shouldRemove) { this.arrayContentWillChange(i, 1, 0); this.currentState.splice(i, 1); this.set('length', this.currentState.length); diff --git a/packages/store/addon/-private/system/model/internal-model.ts b/packages/store/addon/-private/system/model/internal-model.ts index a5447fcc9ac..5aebbcecd23 100644 --- a/packages/store/addon/-private/system/model/internal-model.ts +++ b/packages/store/addon/-private/system/model/internal-model.ts @@ -1395,7 +1395,11 @@ export default class InternalModel { } if (didChange && this.hasRecord) { - this.notifyPropertyChange('id'); + if (CUSTOM_MODEL_CLASS) { + this.store._notificationManager.notify(this.identifier, 'identity'); + } else { + this.notifyPropertyChange('id'); + } } } diff --git a/packages/store/addon/-private/system/record-notification-manager.ts b/packages/store/addon/-private/system/record-notification-manager.ts index fc95b9f79d3..3f8eaf952ad 100644 --- a/packages/store/addon/-private/system/record-notification-manager.ts +++ b/packages/store/addon/-private/system/record-notification-manager.ts @@ -9,7 +9,7 @@ const Tokens = new WeakMap(); interface NotificationCallback { ( identifier: RecordIdentifier, - notificationType: 'attributes' | 'relationships' | 'errors' | 'meta' | 'unload' | 'property' | 'state' + notificationType: 'attributes' | 'relationships' | 'identity' | 'errors' | 'meta' | 'unload' | 'property' | 'state' ): void; } @@ -36,7 +36,7 @@ export default class NotificationManager { notify( identifier: RecordIdentifier, - value: 'attributes' | 'relationships' | 'errors' | 'meta' | 'unload' | 'property' | 'state' + value: 'attributes' | 'relationships' | 'errors' | 'meta' | 'identity' | 'unload' | 'property' | 'state' ): boolean { let stableId = this.store._stableIdentifierFor(identifier); let callback = Cache.get(stableId); diff --git a/packages/store/addon/-private/system/snapshot.js b/packages/store/addon/-private/system/snapshot.js index 447174c7494..db86be98d1f 100644 --- a/packages/store/addon/-private/system/snapshot.js +++ b/packages/store/addon/-private/system/snapshot.js @@ -6,6 +6,7 @@ import EmberError from '@ember/error'; import { get } from '@ember/object'; import { assign } from '@ember/polyfills'; import { relationshipStateFor } from './record-data-for'; +import { CUSTOM_MODEL_CLASS } from '@ember-data/canary-features'; /** @class Snapshot @@ -96,7 +97,17 @@ export default class Snapshot { let record = this.record; attributes = this.__attributes = Object.create(null); let attrs = Object.keys(this._store._attributesDefinitionFor(this.modelName, record.id)); - attrs.forEach(keyName => (attributes[keyName] = get(record, keyName))); + if (CUSTOM_MODEL_CLASS) { + attrs.forEach(keyName => { + if (this.type.isModel) { + attributes[keyName] = get(record, keyName); + } else { + attributes[keyName] = this._internalModel._recordData.getAttr(keyName); + } + }); + } else { + attrs.forEach(keyName => (attributes[keyName] = get(record, keyName))); + } } return attributes; @@ -129,6 +140,9 @@ export default class Snapshot { return this._internalModel.modelClass; } + get isNew() { + return this._internalModel.isNew(); + } /** Returns the value of an attribute.